XHRUploader does not work with responseType: json

[bdirito]

Initial checklist

• I understand this is a bug report and questions should be posted in the Community Forum
• I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

https://stackblitz.com/~/github.com/bdirito/uppyXhruploadJson?file=main.js

Steps to reproduce

Open stackblitz. Open browser console. Upload any file from your machine.

The key aspects of this seem to be:
Create an Uppy instance, set the uploader plugin to XHRUploader. Set XHRUploader's option of responseType to json as specified in https://uppy.io/docs/xhr-upload/#responsetype. Upload a file such that the response is in json format.

Expected behavior

The file will 'successfully' upload, and transition to 'green'. There should be no errors on the console. Callbacks that take an XHR should have xhr.response appropriately parsed.

Actual behavior

The file perpetually sits 'in progress' at '100%'. There following error is printed to console:

InvalidStateError: Failed to read the 'responseText' property from 'XMLHttpRequest': The value is only accessible if the object's 'responseType' is '' or 'text' (was 'json').

The console error is a legit error as via https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseText

I've looked into this and I think the issue is the way https://github.com/transloadit/uppy/blob/main/packages/%40uppy/xhr-upload/src/index.ts handles stuff. From that file on L212 (I've removed some of the fetcher arguments):

         const res = await fetcher(url, {...options, <more stuff>});

          let body = await this.opts.getResponseData?.(res)
          try {
            body ??= JSON.parse(res.responseText) as B
          } catch (cause) {
            throw new Error(
              '@uppy/xhr-upload expects a JSON response (with a `url` property). To parse non-JSON responses, use `getResponseData` to turn your response into JSON.',
              { cause },
            )
          }

We do not have a getResponseData option so we try to parse the XHR's responseText in uppy. But since the XHR has set its responseType to json, responseText is not available. In fact the whole logic here seems 'impossible'; the only way we could have responseText available is if responseType was either '' or 'text'. However in such a case you presumably should not be attempting to parse the response as json. On the other hand if the response is json you have no need to 're-parse' the data (and can not access responseText).

I did not further analyze why the upload is still 'in progress' - I assume its an artifact of the throw. Though even to the extent the throw was 'legitimate' I would expect the upload to be aborted with an error.

Workarounds

• Set responseType to '' or 'text'. (but actually return json). This means that you will have to JSON.parse(xrh.responseText) yourself for any apis that you may want to use that are passing in xhr (for example shouldRetry or onAfterResponse).
• Recommended: Set getResponseData to a noop function.

      getResponseData: (xhr: XMLHttpRequest) => {
        return xhr.response as UppyBody; // UppyBody is my type passed into new Uppy<UppyMeta, UppyBody>({...})
      },

[Murderlon]

I'll take a look when I can

Note that you don't need to do anything for the workaround. responseType is '' by default and we also JSON.parse() by default. No options need to be set to send back JSON.

[bdirito]

The issue with 'just using the default responseType' is that the xhrs that are passed around to various apis such as shouldRetry or onAfterResponse get that default '' responseType which means that as far as they are concerned what you are actually getting is text meaning that you then have to then run a JSON.parse() yourself in those apis. Not a huge deal to just parse yourself, but if the api to set the response type to json is part of the exposed api, then it needs to work.

I do not know what the best way to fix this would be. Perhaps the 'cleanest' and 'simplest' approach would simply to change the default 'responseType' to 'json' as XHRPlugin seems to implicitly want 'json' responses; there is an explicit function to 'turn your non-JSON response into JSON' (https://uppy.io/docs/xhr-upload/#getresponsedata). However this would break any library user who was then trying to access xhr.responseText in a callback function such as shouldRetry(xhr).

While this plugin seems to assume the use of JSON formatted response data the underlying use of body variable seems to be to feed it to the upload-success event; which is much more agnostic about what stuff should look like; there is no explicit callout that it should be json at all https://uppy.io/docs/uppy/#upload-success

Thus what I think I'd suggest is that XHRUpload.#getFetcher 'res.response' (xhr.response) is 'normally' simply assigned to body. This would be true for everything except the cases where responseType is set to '', 'text' and unspecified. In such cases it is explained in the documentation that since these values predate the availability of 'json' (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType#json) the default behavior of Uppy is to use 'text' and treat the response as json, and explain if you actually want to use textual response data you should override the getResponseData() function.

Furthermore the getResponseData documentation would be updated to suggest that It is more generic then just converting to JSON; you can convert data of whatever responseType to your specified Uppy Body type.

Proposed documentation:

---

responseType
The response type expected from the server, determining how the xhr.response property should be filled (string, default: 'text').

This option sets the XMLHttpRequest.responseType property of the underlying XMLHttpRequest used for uploads. This xhr can be accessed in various callback functions such as getResponseData().

The default expectation of responses from your upload server is JSON data. However due to the json responseType being newer then '' and 'text', Uppy defaults to 'text' and treats it as json. If you wish to override this and actually return non-JSON text from your api you should provide a getResponseData() to return the actual unparsed text data.

---

getResponseData
An optional function to pre-process the response data for availability in Uppy event callbacks (such as 'upload-success' https://uppy.io/docs/uppy/#upload-success).

The response type should match your Uppy 'Body' specification. Ie new Uppy<Meta, Body>(...).

If your response happens to be an object extending {url: string} url will be extracted out for use in the upload-success event. Otherwise upload-success will set it's uploadURL to undefined.

[bdirito]

I should be able to open a PR for this (depending on exactly what fix is wanted). However I don't know that I have the schedule to do this anytime 'quick'.

[Murderlon]

Thanks for all the details. I don't think we can switch the default responseType as some people return XML or something else and use getResponseData to extract it.

I'll think about what makes the most sense.