companion: fix tus stream source and s3 region fallback regressions

packages/@uppy/companion/__mocks__/tus-js-client.ts

@@ -4,12 +4,23 @@ type UploadOptions = {
   onSuccess: () => void
 } & Record<string, unknown>
 
+let lastUploadFile: unknown
+
+export function __getLastUploadFile(): unknown {
+  return lastUploadFile
+}
+
+export function __resetTusMockState(): void {
+  lastUploadFile = undefined
+}
+
 export class Upload {
   url: string
 
   options: UploadOptions
 
-  constructor(_file: unknown, options: UploadOptions) {
+  constructor(file: unknown, options: UploadOptions) {
+    lastUploadFile = file
     this.url = 'https://tus.endpoint/files/foo-bar'
     this.options = options
   }

packages/@uppy/companion/src/server/Uploader.ts

@@ -3,7 +3,7 @@ import { createReadStream, createWriteStream, ReadStream } from 'node:fs'
 import { stat, unlink } from 'node:fs/promises'
 import { join } from 'node:path'
 import type { Readable as NodeReadableStream } from 'node:stream'
-import { Readable as NodeReadable, Transform } from 'node:stream'
+import { Transform } from 'node:stream'
 import { pipeline } from 'node:stream/promises'
 import type { PutObjectCommandInput, S3Client } from '@aws-sdk/client-s3'
 import { Upload } from '@aws-sdk/lib-storage'
@@ -803,19 +803,7 @@ export default class Uploader {
         tusOptions.uploadSize = this.size
       }
 
-      const nodeReader = NodeReadable.toWeb(stream).getReader()
-      const reader: Pick<ReadableStreamDefaultReader<unknown>, 'read'> = {
-        async read() {
-          const { done, value } = await nodeReader.read()
-          // Ensure we always provide `value`, since some ReadableStream lib variants
-          // type it differently when `done: true`.
-          return done
-            ? { done: true, value: undefined }
-            : { done: false, value }
-        },
-      }
-
-      this.tus = new tus.Upload(reader, tusOptions)
+      this.tus = new tus.Upload(stream, tusOptions)
 
       this.tus.start()
     })

packages/@uppy/companion/src/server/s3-client.ts

@@ -32,6 +32,10 @@ export default function s3Client(
       const v = obj[key]
       return typeof v === 'boolean' ? v : undefined
     }
+    const isRegion = (
+      value: unknown,
+    ): value is NonNullable<S3ClientConfig['region']> =>
+      typeof value === 'string' || typeof value === 'function'
 
     if (s3['accessKeyId'] || s3['secretAccessKey']) {
       throw new Error(
@@ -51,25 +55,23 @@ export default function s3Client(
       )
     }
 
-    const region =
-      getString(s3, 'region') ??
-      (rawClientOptions && typeof rawClientOptions['region'] === 'string'
-        ? rawClientOptions['region']
-        : undefined)
-    if (!region) {
-      // No region means this Companion instance isn't configured for S3 uploads.
-      return null
-    }
+    const explicitRegion = getString(s3, 'region')
+    const rawClientRegion = rawClientOptions?.['region']
+    const region = explicitRegion ?? (isRegion(rawClientRegion) ? rawClientRegion : undefined)
 
-    let endpoint: unknown = s3.endpoint
-    if (typeof endpoint === 'string') {
+    let endpoint: string | undefined =
+      typeof s3.endpoint === 'string' ? s3.endpoint : undefined
+    if (endpoint) {
       // TODO: deprecate those replacements in favor of what AWS SDK supports out of the box.
-      endpoint = endpoint.replace(/{service}/, 's3').replace(/{region}/, region)
+      endpoint = endpoint.replace(/{service}/, 's3')
+      if (explicitRegion) {
+        endpoint = endpoint.replace(/{region}/, explicitRegion)
+      }
     }
 
     const forcePathStyle = getBool(s3, 'forcePathStyle')
     let s3ClientOptions: S3ClientConfig = {
-      region,
+      ...(region === undefined ? {} : { region }),
       ...(forcePathStyle === undefined ? {} : { forcePathStyle }),
     }
 
@@ -96,7 +98,7 @@ export default function s3Client(
       }
     } else {
       // no accelearate, we allow custom s3 endpoint
-      if (typeof endpoint === 'string') {
+      if (endpoint !== undefined) {
         s3ClientOptions = {
           ...s3ClientOptions,
           endpoint,
@@ -124,7 +126,15 @@ export default function s3Client(
         ...(sessionToken ? { sessionToken } : {}),
       }
     }
-    s3Client = new S3Client(s3ClientOptions)
+    try {
+      s3Client = new S3Client(s3ClientOptions)
+    } catch (err) {
+      // Keep S3 optional when no region can be resolved at all.
+      if (err instanceof Error && err.message === 'Region is missing') {
+        return null
+      }
+      throw err
+    }
   }
 
   return s3Client

packages/@uppy/companion/test/s3-client.test.ts

@@ -0,0 +1,51 @@
+import { afterEach, describe, expect, test } from 'vitest'
+import { defaultOptions } from '../src/config/companion.ts'
+import s3Client from '../src/server/s3-client.ts'
+import type { CompanionRuntimeOptions } from '../src/types/companion-options.ts'
+
+const originalAwsRegion = process.env['AWS_REGION']
+
+afterEach(() => {
+  if (originalAwsRegion === undefined) {
+    delete process.env['AWS_REGION']
+  } else {
+    process.env['AWS_REGION'] = originalAwsRegion
+  }
+})
+
+describe('s3-client', () => {
+  test('creates an S3 client when region comes from AWS SDK environment resolution', () => {
+    process.env['AWS_REGION'] = 'us-east-1'
+
+    const options = {
+      ...defaultOptions,
+      s3: {
+        ...defaultOptions.s3,
+        endpoint: 'https://s3.amazonaws.com',
+        bucket: 'test-bucket',
+        key: 'test-key',
+        secret: 'test-secret',
+      },
+    } satisfies CompanionRuntimeOptions
+
+    expect(s3Client(options)).not.toBeNull()
+  })
+
+  test('creates an S3 client when region is provided as awsClientOptions provider', () => {
+    const options = {
+      ...defaultOptions,
+      s3: {
+        ...defaultOptions.s3,
+        endpoint: 'https://s3.amazonaws.com',
+        bucket: 'test-bucket',
+        key: 'test-key',
+        secret: 'test-secret',
+        awsClientOptions: {
+          region: async () => 'us-east-1',
+        },
+      },
+    } satisfies CompanionRuntimeOptions
+
+    expect(s3Client(options)).not.toBeNull()
+  })
+})

packages/@uppy/companion/test/uploader.test.ts

@@ -4,6 +4,7 @@ import type { Request } from 'express'
 import express from 'express'
 import nock from 'nock'
 import request from 'supertest'
+import * as tusClient from 'tus-js-client'
 import {
   afterAll,
   afterEach,
@@ -24,8 +25,25 @@ import * as socketClient from './mocksocket.ts'
 vi.mock('tus-js-client')
 vi.mock('express-prom-bundle')
 
+function getTusMockLastUploadFile(): unknown {
+  const getter = Reflect.get(tusClient, '__getLastUploadFile')
+  if (typeof getter !== 'function') {
+    throw new Error('Expected tus mock to expose __getLastUploadFile()')
+  }
+  return getter()
+}
+
+function resetTusMockState(): void {
+  const reset = Reflect.get(tusClient, '__resetTusMockState')
+  if (typeof reset !== 'function') {
+    throw new Error('Expected tus mock to expose __resetTusMockState()')
+  }
+  reset()
+}
+
 afterEach(() => {
   nock.cleanAll()
+  resetTusMockState()
 })
 afterAll(() => {
   nock.restore()
@@ -162,6 +180,7 @@ describe('uploader', () => {
     socketClient.onUploadSuccess(uploadToken, onUploadSuccess)
     await promise
     await uploader.tryUploadStream(stream, requireMockReq())
+    expect(getTusMockLastUploadFile()).toBeInstanceOf(Readable)
 
     expect(onUploadError).not.toHaveBeenCalled()