transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
28.88k stars 1.98k forks source link

XHR plugin expects upload response to be a valid JSON #5349

Closed aonnikov closed 1 month ago

aonnikov commented 1 month ago

Initial checklist

Link to runnable example

No response

Steps to reproduce

XHR plugin in Uppy 4.0 seem to require the file upload endpoint to return a valid JSON. There is a code that attempts to extract upload URL from the response:

          const body = JSON.parse(res.responseText) as B
          const uploadURL = typeof body?.url === 'string' ? body.url : undefined

If the response is not a JSON document, then upload fails.

Expected behavior

It is expected that this behavior is configurable or the code has proper error handling.

Actual behavior

File upload fails. I figured out that responseType option not used. This might be a regression introduced after migrating to fetcher use.

jaswindersingh1903 commented 1 month ago

Do you think this is the same issue I have https://github.com/transloadit/uppy/issues/5328

aonnikov commented 1 month ago

Nope, to me this does not look like the same issue.

Murderlon commented 1 month ago

Hi, what do you want to return that is not JSON? We made it required because Uppy (ideally) needs to know where the resource that was just uploaded is located.

If you return XML, you could use onAfterResponse to return JSON with a url property.

aonnikov commented 1 month ago

My particular service returns plain text with id of the uploaded file and I have no intention to change the response only to make Uppy happy. With 3.x it works just fine because there is the getResponseData that allows to parse response in any format and return body. In 4.0 there is no way to do this, because Uppy assumes response to be a JSON document; onAfterResponse returns void.

Murderlon commented 1 month ago

I can make a change to allow onAfterResponse to return a different body

aonnikov commented 1 month ago

As I understand, upload location URL is needed for Golden Retriever, that makes sense ... Is that possible to keep the same behavior as in Uppy 3.x? I.e. if getResponseData is provided, it is used to parse the response, otherwise Uppy attempts to parse it as a JSON document. Proper error handling is important here as well, it took me some time to realize that upload failed due to unsupported type of content returned by XHR upload service.

Murderlon commented 1 month ago

I will log a better error but getResponseData has been removed and ideally we don't bring it back. The problem with the previous hooks was is that people want to do all kinds of things and they were overly specific. But we can use onAfterResponse to cover your use case with some changes.

aonnikov commented 1 month ago

I'm new here and was not aware of problems users or devs run into with previous hooks. I started with 3.x version and was happy to see 4.x with improved typescript support, but the problem with XHR uploader forced me to stay on 3.x at least for now. From my point of view, requirement to return a document in a specific format is very strict limitation, and makes the XHR uploader very specific, whereas it is supposed to be a general purpose uploader. Users may not want or may not have an ability to adjust response format of their upload endpoints.