Type results of slicing and HTTP request content

Author: Acconut

lib/browser/FetchHttpStack.ts

@@ -1,4 +1,11 @@
-import type { HttpProgressHandler, HttpRequest, HttpResponse, HttpStack } from '../options.js'
+import { readable as isNodeReadableStream } from 'is-stream'
+import type {
+  HttpProgressHandler,
+  HttpRequest,
+  HttpResponse,
+  HttpStack,
+  SliceType,
+} from '../options.js'
 
 // TODO: Add tests for this.
 export class FetchHttpStack implements HttpStack {
@@ -42,7 +49,13 @@ class FetchRequest implements HttpRequest {
     // The Fetch API currently does not expose a way to track upload progress.
   }
 
-  async send(body?: Blob): Promise<FetchResponse> {
+  async send(body?: SliceType): Promise<FetchResponse> {
+    if (isNodeReadableStream(body)) {
+      throw new Error(
+        'Using a Node.js readable stream as HTTP request body is not supported using the Fetch API HTTP stack.',
+      )
+    }
+
     const res = await fetch(this._url, {
       method: this._method,
       headers: this._headers,

lib/node/NodeHttpStack.ts

@@ -4,9 +4,15 @@ import * as http from 'node:http'
 import * as https from 'node:https'
 import { Readable, Transform, type Writable } from 'node:stream'
 import { parse } from 'node:url'
+import isStream from 'is-stream'
 import throttle from 'lodash.throttle'
-import type { HttpProgressHandler, HttpRequest, HttpResponse, HttpStack } from '../options.js'
-import type { FileSliceTypes } from './index.js'
+import type {
+  HttpProgressHandler,
+  HttpRequest,
+  HttpResponse,
+  HttpStack,
+  SliceType,
+} from '../options.js'
 
 export class NodeHttpStack implements HttpStack {
   private _requestOptions: http.RequestOptions
@@ -63,9 +69,25 @@ class Request implements HttpRequest {
     this._progressHandler = progressHandler
   }
 
-  async send(body?: FileSliceTypes): Promise<HttpResponse> {
-    if (body instanceof Blob) {
-      body = await body.arrayBuffer()
+  async send(body?: SliceType): Promise<HttpResponse> {
+    let nodeBody: Readable | Uint8Array | undefined
+    if (body != null) {
+      if (body instanceof Blob) {
+        nodeBody = new Uint8Array(await body.arrayBuffer())
+      } else if (body instanceof Uint8Array) {
+        nodeBody = body
+      } else if (ArrayBuffer.isView(body)) {
+        // Any typed array other than Uint8Array or a DataVew
+        nodeBody = new Uint8Array(body.buffer, body.byteOffset, body.byteLength)
+      } else if (isStream.readable(body)) {
+        nodeBody = body
+      } else {
+        throw new Error(
+          // @ts-expect-error According to the types, this case cannot happen. But
+          // we still want to try logging the constructor if this code is reached by accident.
+          `Unsupported HTTP request body type in Node.js HTTP stack: ${typeof body} (constructor: ${body?.constructor?.name})`,
+        )
+      }
     }
 
     return new Promise((resolve, reject) => {
@@ -106,27 +128,19 @@ class Request implements HttpRequest {
         reject(err)
       })
 
-      if (body instanceof ArrayBuffer || body instanceof SharedArrayBuffer) {
-        body = new Uint8Array(body)
-      }
-
-      if (ArrayBuffer.isView(body) && !(body instanceof Uint8Array)) {
-        body = new Uint8Array(body.buffer, body.byteOffset, body.byteLength)
-      }
-
-      if (body instanceof Readable) {
+      if (nodeBody instanceof Readable) {
         // Readable stream are piped through a PassThrough instance, which
         // counts the number of bytes passed through. This is used, for example,
         // when an fs.ReadStream is provided to tus-js-client.
-        body.pipe(new ProgressEmitter(this._progressHandler)).pipe(req)
-      } else if (body instanceof Uint8Array) {
+        nodeBody.pipe(new ProgressEmitter(this._progressHandler)).pipe(req)
+      } else if (nodeBody instanceof Uint8Array) {
         // For Buffers and Uint8Arrays (in Node.js all buffers are instances of Uint8Array),
         // we write chunks of the buffer to the stream and use that to track the progress.
         // This is used when either a Buffer or a normal readable stream is provided
         // to tus-js-client.
-        writeBufferToStreamWithProgress(req, body, this._progressHandler)
+        writeBufferToStreamWithProgress(req, nodeBody, this._progressHandler)
       } else {
-        req.end(body)
+        req.end()
       }
     })
   }

lib/node/index.ts

@@ -1,9 +1,7 @@
-import type { ReadStream } from 'node:fs'
-import type { Readable } from 'node:stream'
 import { DetailedError } from '../DetailedError.js'
 import { NoopUrlStorage } from '../NoopUrlStorage.js'
 import { enableDebugLog } from '../logger.js'
-import type { PathReference, UploadInput, UploadOptions } from '../options.js'
+import type { UploadInput, UploadOptions } from '../options.js'
 import { BaseUpload, defaultOptions as baseDefaultOptions, terminate } from '../upload.js'
 
 import { canStoreURLs } from './FileUrlStorage.js'
@@ -19,16 +17,6 @@ const defaultOptions = {
   fingerprint,
 }
 
-export type FileTypes =
-  | ArrayBuffer
-  | SharedArrayBuffer
-  | ArrayBufferView
-  | Blob
-  | Readable
-  | PathReference
-
-export type FileSliceTypes = ArrayBuffer | SharedArrayBuffer | ArrayBufferView | Blob | ReadStream
-
 class Upload extends BaseUpload {
   constructor(file: UploadInput, options: Partial<UploadOptions> = {}) {
     const allOpts = { ...defaultOptions, ...options }

lib/node/sources/PathFileSource.ts

@@ -35,6 +35,8 @@ export class PathFileSource implements FileSource {
   }
 
   slice(start: number, end: number) {
+    // TODO: Does this create multiple file descriptors? Can we reduce this by
+    // using a file handle instead?
     // The path reference might be configured to not read from the beginning,
     // but instead start at a different offset. The start value from the caller
     // does not include the offset, so we need to add this offset to our range later.

lib/options.ts

@@ -1,4 +1,4 @@
-import type { Readable } from 'node:stream'
+import type { Readable as NodeReadableStream } from 'node:stream'
 import type { DetailedError } from './DetailedError.js'
 
 export const PROTOCOL_TUS_V1 = 'tus-v1'
@@ -36,17 +36,17 @@ export interface PathReference {
 }
 
 export type UploadInput =
-  // Blob, File, ReadableStreamDefaultReader are available in browsers and Node.js
+  // available in all environments
   | Blob // includes File
   | ArrayBuffer
   | SharedArrayBuffer
   | ArrayBufferView // includes Node.js' Buffer
-  // TODO: Should we keep the Pick<> here?
-  // TODO: Should we also accept ReadableStreamBYOBReader? What about ReadableStream?
+  // TODO: We should accept a ReadableStream instead of a reader
   | Pick<ReadableStreamDefaultReader, 'read'>
-  // Buffer, stream.Readable, fs.ReadStream are available in Node.js
-  | Readable // TODO: Replace this with our own interface based on https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3634b01d50c10ce1afaae63e41d39e7da309d8e3/types/node/globals.d.ts#L399
+  // available in Node.js
+  | NodeReadableStream
   | PathReference
+  // available in React Native
   | ReactNativeFile
 
 export interface UploadOptions {
@@ -120,6 +120,9 @@ export interface FileSource {
   close(): void
 }
 
+// TODO: Allow Web Streams' ReadableStream as well
+export type SliceType = Blob | ArrayBufferView | NodeReadableStream
+
 export type SliceResult =
   | {
       done: true
@@ -128,9 +131,11 @@ export type SliceResult =
     }
   | {
       done: boolean
-      // Platform-specific data type which must be usable by the HTTP stack as a body.
-      // TODO: This shouldn't be unknown, but precise values.
-      value: NonNullable<unknown>
+      value: NonNullable<SliceType>
+      // TODO: How should sizes be handled? If we want to allow `slice()` to return
+      // streams without buffering them before, this cannot return a known size.
+      // Should size be returned by the HTTP stack based on the number of uploaded bytes?
+      // It would make sense since it likely already counts progress.
       size: number
     }
 
@@ -150,7 +155,7 @@ export interface HttpRequest {
 
   setProgressHandler(handler: HttpProgressHandler): void
   // TODO: Should this be something like { value: unknown, size: number }?
-  send(body?: unknown): Promise<HttpResponse>
+  send(body?: SliceType): Promise<HttpResponse>
   abort(): Promise<void>
 
   // Return an environment specific object, e.g. the XMLHttpRequest object in browsers.

lib/upload.ts

@@ -12,6 +12,7 @@ import {
   PROTOCOL_IETF_DRAFT_05,
   PROTOCOL_TUS_V1,
   type PreviousUpload,
+  type SliceType,
   type UploadInput,
   type UploadOptions,
 } from './options.js'
@@ -981,7 +982,7 @@ export class BaseUpload {
    *
    * @api private
    */
-  _sendRequest(req: HttpRequest, body?: unknown): Promise<HttpResponse> {
+  _sendRequest(req: HttpRequest, body?: SliceType): Promise<HttpResponse> {
     return sendRequest(req, body, this.options)
   }
 }
@@ -1041,7 +1042,7 @@ function openRequest(method: string, url: string, options: UploadOptions): HttpR
  */
 async function sendRequest(
   req: HttpRequest,
-  body: unknown | undefined,
+  body: SliceType | undefined,
   options: UploadOptions,
 ): Promise<HttpResponse> {
   if (typeof options.onBeforeRequest === 'function') {