@uppy/xhr-upload: introduce hooks similar to tus

packages/@uppy/utils/src/fetcher.ts

@@ -38,7 +38,7 @@ export type FetcherOptions = {
   shouldRetry?: (xhr: XMLHttpRequest) => boolean
 
   /** Called after the response has succeeded or failed but before the promise is resolved. */
-  onAfterRequest?: (
+  onAfterResponse?: (
     xhr: XMLHttpRequest,
     retryCount: number,
   ) => void | Promise<void>
@@ -67,7 +67,7 @@ export function fetcher(
     onBeforeRequest = noop,
     onUploadProgress = noop,
     shouldRetry = () => true,
-    onAfterRequest = noop,
+    onAfterResponse = noop,
     onTimeout = noop,
     responseType,
     retries = 3,
@@ -99,7 +99,7 @@ export function fetcher(
       })
 
       xhr.onload = async () => {
-        await onAfterRequest(xhr, retryCount)
+        await onAfterResponse(xhr, retryCount)
 
         if (xhr.status >= 200 && xhr.status < 300) {
           timer.done()

packages/@uppy/xhr-upload/src/index.test.ts

@@ -4,88 +4,44 @@ import Core from '@uppy/core'
 import XHRUpload from './index.ts'
 
 describe('XHRUpload', () => {
-  describe('getResponseData', () => {
-    it('has the XHRUpload options as its `this`', () => {
-      nock('https://fake-endpoint.uppy.io')
-        .defaultReplyHeaders({
-          'access-control-allow-method': 'POST',
-          'access-control-allow-origin': '*',
-        })
-        .options('/')
-        .reply(200, {})
-        .post('/')
-        .reply(200, {})
-
-      const core = new Core()
-      const getResponseData = vi.fn(function getResponseData() {
-        // @ts-expect-error TS can't know the type
-        expect(this.some).toEqual('option')
-        return {}
-      })
-      core.use(XHRUpload, {
-        id: 'XHRUpload',
-        endpoint: 'https://fake-endpoint.uppy.io',
-        // @ts-expect-error that option does not exist
-        some: 'option',
-        getResponseData,
-      })
-      core.addFile({
-        type: 'image/png',
-        source: 'test',
-        name: 'test.jpg',
-        data: new Blob([new Uint8Array(8192)]),
-      })
-
-      return core.upload().then(() => {
-        expect(getResponseData).toHaveBeenCalled()
+  it('should leverage hooks from fetcher', () => {
+    nock('https://fake-endpoint.uppy.io')
+      .defaultReplyHeaders({
+        'access-control-allow-method': 'POST',
+        'access-control-allow-origin': '*',
       })
-    })
-  })
+      .options('/')
+      .reply(204, {})
+      .post('/')
+      .reply(401, {})
+      .options('/')
+      .reply(204, {})
+      .post('/')
+      .reply(200, {})
 
-  describe('validateStatus', () => {
-    it('emit upload error under status code 200', () => {
-      nock('https://fake-endpoint.uppy.io')
-        .defaultReplyHeaders({
-          'access-control-allow-method': 'POST',
-          'access-control-allow-origin': '*',
-        })
-        .options('/')
-        .reply(200, {})
-        .post('/')
-        .reply(200, {
-          code: 40000,
-          message: 'custom upload error',
-        })
+    const core = new Core()
+    const shouldRetry = vi.fn(() => true)
+    const onBeforeRequest = vi.fn(() => {})
+    const onAfterResponse = vi.fn(() => {})
 
-      const core = new Core()
-      const validateStatus = vi.fn((status, responseText) => {
-        return JSON.parse(responseText).code !== 40000
-      })
-
-      core.use(XHRUpload, {
-        id: 'XHRUpload',
-        endpoint: 'https://fake-endpoint.uppy.io',
-        // @ts-expect-error that option doesn't exist
-        some: 'option',
-        validateStatus,
-        getResponseError(responseText) {
-          return JSON.parse(responseText).message
-        },
-      })
-      core.addFile({
-        type: 'image/png',
-        source: 'test',
-        name: 'test.jpg',
-        data: new Blob([new Uint8Array(8192)]),
-      })
+    core.use(XHRUpload, {
+      id: 'XHRUpload',
+      endpoint: 'https://fake-endpoint.uppy.io',
+      shouldRetry,
+      onBeforeRequest,
+      onAfterResponse,
+    })
+    core.addFile({
+      type: 'image/png',
+      source: 'test',
+      name: 'test.jpg',
+      data: new Blob([new Uint8Array(8192)]),
+    })
 
-      return core.upload().then((result) => {
-        expect(validateStatus).toHaveBeenCalled()
-        expect(result!.failed!.length).toBeGreaterThan(0)
-        result!.failed!.forEach((file) => {
-          expect(file.error).toEqual('custom upload error')
-        })
-      })
+    return core.upload().then(() => {
+      expect(shouldRetry).toHaveBeenCalledTimes(1)
+      expect(onAfterResponse).toHaveBeenCalledTimes(2)
+      expect(onBeforeRequest).toHaveBeenCalledTimes(2)
     })
   })
 

packages/@uppy/xhr-upload/src/index.ts

@@ -10,7 +10,7 @@ import {
 } from '@uppy/utils/lib/RateLimitedQueue'
 import NetworkError from '@uppy/utils/lib/NetworkError'
 import isNetworkError from '@uppy/utils/lib/isNetworkError'
-import { fetcher } from '@uppy/utils/lib/fetcher'
+import { fetcher, type FetcherOptions } from '@uppy/utils/lib/fetcher'
 import {
   filterNonFailedFiles,
   filterFilesToEmitUploadStarted,
@@ -54,16 +54,11 @@ export interface XhrUploadOpts<M extends Meta, B extends Body>
   limit?: number
   responseType?: XMLHttpRequestResponseType
   withCredentials?: boolean
-  validateStatus?: (
-    status: number,
-    body: string,
-    xhr: XMLHttpRequest,
-  ) => boolean
-  getResponseData?: (body: string, xhr: XMLHttpRequest) => B
-  getResponseError?: (body: string, xhr: XMLHttpRequest) => Error | NetworkError
-  allowedMetaFields?: string[] | boolean
+  onBeforeRequest?: FetcherOptions['onBeforeRequest']
+  shouldRetry?: FetcherOptions['shouldRetry']
+  onAfterResponse?: FetcherOptions['onAfterResponse']
+  allowedMetaFields?: boolean | string[]
   bundle?: boolean
-  responseUrlFieldName?: string
 }
 
 function buildResponseError(
@@ -106,37 +101,12 @@ const defaultOptions = {
   fieldName: 'file',
   method: 'post',
   allowedMetaFields: true,
-  responseUrlFieldName: 'url',
   bundle: false,
   headers: {},
   timeout: 30 * 1000,
   limit: 5,
   withCredentials: false,
   responseType: '',
-  getResponseData(responseText) {
-    let parsedResponse = {}
-    try {
-      parsedResponse = JSON.parse(responseText)
-    } catch {
-      // ignore
-    }
-    // We don't have access to the B (Body) generic here
-    // so we have to cast it to any. The user facing types
-    // remain correct, this is only to please the merging of default options.
-    return parsedResponse as any
-  },
-  getResponseError(_, response) {
-    let error = new Error('Upload error')
-
-    if (isNetworkError(response)) {
-      error = new NetworkError(error, response)
-    }
-
-    return error
-  },
-  validateStatus(status) {
-    return status >= 200 && status < 300
-  },
 } satisfies Partial<XhrUploadOpts<any, any>>
 
 type Opts<M extends Meta, B extends Body> = DefinePluginOpts<
@@ -215,6 +185,9 @@ export default class XHRUpload<
         try {
           const res = await fetcher(url, {
             ...options,
+            onBeforeRequest: this.opts.onBeforeRequest,
+            shouldRetry: this.opts.shouldRetry,
+            onAfterResponse: this.opts.onAfterResponse,
             onTimeout: (timeout) => {
               const seconds = Math.ceil(timeout / 1000)
               const error = new Error(this.i18n('uploadStalled', { seconds }))
@@ -235,23 +208,19 @@ export default class XHRUpload<
             },
           })
 
-          if (!this.opts.validateStatus(res.status, res.responseText, res)) {
-            throw new NetworkError(res.statusText, res)
-          }
+          const body = JSON.parse(res.responseText)
 
-          const body = this.opts.getResponseData(res.responseText, res)
-          const uploadURL = body[this.opts.responseUrlFieldName]
-          if (typeof uploadURL !== 'string') {
+          if (!body?.url) {
             throw new Error(
-              `The received response did not include a valid URL for key ${this.opts.responseUrlFieldName}`,
+              'Expected body to be JSON and have a `url` property.',
             )
           }
 
           for (const file of files) {
             this.uppy.emit('upload-success', file, {
               status: res.status,
               body,
-              uploadURL,
+              uploadURL: body.url,
             })
           }
 
@@ -262,12 +231,13 @@ export default class XHRUpload<
           }
           if (error instanceof NetworkError) {
             const request = error.request!
-            const customError = buildResponseError(
-              request,
-              this.opts.getResponseError(request.responseText, request),
-            )
+
             for (const file of files) {
-              this.uppy.emit('upload-error', file, customError)
+              this.uppy.emit(
+                'upload-error',
+                file,
+                buildResponseError(request, error),
+              )
             }
           }