[Meta] Multipart requests need rework

[tajnymag]

Describe the bug
Right now, binary multipart/form-data requests almost always fail to work.

Issues:

• no way to upload files: Question: is there a way to override the generated Blob type to be File? ferdikoomen/openapi-typescript-codegen#1825
• axios rejecting generated formdata object: getFormData causes issue in submitting files ferdikoomen/openapi-typescript-codegen#1989
• malformed form data serialization: Issue with Multipart Form Data Serialization in openapi-typescript-codegen ferdikoomen/openapi-typescript-codegen#1804
• client processing incorrect form fields:
  • fix: Object.entries(formData) in getFormData ferdikoomen/openapi-typescript-codegen#1894
  • (FormData) TypeError: source.on is not a function ferdikoomen/openapi-typescript-codegen#1941

In my testing, I've used example requests from Swagger documentation and compared the result to the official java-based typescript-axios generator. The official generator correctly inferred the requests were containing files, made the operation input be File instead of Blob and correctly serialized and performed the request.

[mrlubos]

Thanks for reporting @tajnymag. I've got a number of other issues to prioritise first, can visit this afterwards

[Darhagonable]

+1
I'm working with multipart requests now as well.

[jordanshatford]

Related to: (FormData) TypeError: source.on is not a function

see: form-data/form-data#529

The library we are using does not support appending blobs to FormData (axios/node-fetch). We could switch to using build in FormData, but when running axios in node it would be experimental (until node 21) see: here look at history section

cc: @mrlubos

[jordanshatford]

@mrlubos I think a config option to use File instead of Blob makes the most since (File extends Blob). What are your thoughts and what would you name this option? Maybe useFileForBlob?

[tajnymag]

@mrlubos I think a config option to use File instead of Blob makes the most since (File extends Blob). What are your thoughts and what would you name this option? Maybe useFileForBlob?

Such option would override form types across the whole spec? That wouldn't allow to support projects with mixed forms.

If the feature should be hidden behind an option, perhaps allowFileType, might be more universal. Or, if the limitation is tied solely to Node.js version, how about useNodeNext, which would probably be useful for other future features depending on new APIs.

Also, is it necessary for all clients to support the same feature set?

[jordanshatford]

@tajnymag we would like to have all clients support this. "File" appears to be supported in node 18+ so no issues there. As that's the minimum version we officially support. The option would make all Blob turn into File. Not sure how exactly we could support it otherwise. I don't think OpenAPI has a way of differing between those.

Aside: working on fixing all the other bugs, maybe appear to be axios only now. But I have solution. Just need to decide how we want to support File type with mrlubos

[tajnymag]

@jordanshatford You are right, that the spec does not have any explicit file identifier. The codegen internally doesn't even differentiate between binary and file (line from codegen). However, some generators do try make some assumptions. I'll try to go through them and try to understand their logic.

Could the API accept both objects than? And if the given object is an instance of a file (or perhaps even more generally, if the object has a name attribute, to be considered a File) the client would include the filename into the form. It would be spec compliant, not too taxing on implementation and would support mixed projects.

I really think having every binary be exclusively File OR Blob to be quite limiting.

[jordanshatford]

@mrlubos thoughts on changing this to File | Blob? User can narrow the type if they want to check the return value being specifically a file. For sending requests File and Blob are both accepted in the form data

[mrlubos]

That sounds reasonable @jordanshatford

[jordanshatford]

#186 Will use Blob | File for parameter and return typing. The user can narrow the type on there own.

[jordanshatford]

All above issues should be fixed in v0.32.1 of the package. Please test it out and report and issues you find under a new issue. :) Thanks

[mrlubos]

Amazing work @jordanshatford!

[muka]

Hi, I am trying this with node v20.x uploading to a remote server.

The generated client uses axios and has multipart formData.

I tried to populate the object with fields and a blob, but server side I got an empty file upload.

This ended up working

    // NOTE *not* using 'form-data' or it fails with 'TypeError: source.on is not a function'
    const formData = new FormData();
    formData.append("file", file); // Blob
    for (const key in model) {
      formData.append(key, model[key]);
    }

   await client.ui.saveBackground({
    formData: formData as any,
  }),

[jordanshatford]

@muka can you try with the latest version (released early today) which fixed a few issues with axios. v0.33.2. If you are still experiencing issues, please submit a bug report and fill out as much detail as possible.

[muka]

Thank you, just tried but axios is not happy yet

my client package.json

"axios": "^1.6.8",
"@hey-api/openapi-ts": "^0.33.2",

calling the generated api with a blob (a previously described) result in this. Using a Buffer result in an empty file server side.

          new Blob([await readFile(filename)], {
            type: lookup(filename),
          }),

the error from the client (but seems from axios actually)

 AxiosError: Blob is not supported. Use a Buffer instead.
    at convertValue (.../node_modules/axios/lib/helpers/toFormData.js:124:13)
    at FormData.defaultVisitor (.../node_modules/axios/lib/helpers/toFormData.js:175:49)
    ...

Using a FormData as input still works. Why not just allow FormData type for node+axios ?

[jordanshatford]

@muka can you create a new issue and file out all the bug details. that way it will be easier to reproduce