openapi-typescript-codegen icon indicating copy to clipboard operation
openapi-typescript-codegen copied to clipboard

Fix binary response type to process as a Blob rather than text/string

Open loganbenjamin opened this issue 3 years ago • 27 comments

Issue When the OpenAPI response type is a binary type the generated TypeScript has a response type of Promise<Blob> but the response returned is a string. E.g. for fetch the code is return await response.text(). This mismatches from the expected type defined in the TypeScript Blob vs actual type string, and can corrupt the data.

Solution Update the code to detect when response type is a binary type and return a Blob type. E.g. for fetch return await response.blob().

Changes

  • Update ApiRequestOptions type to include a responseType parameter.
  • Update to detect if the OpenAPI response type is a binary type and set the request responseType = 'blob'. This is the same way the TypeScript definition is generated currently.
  • Update request code to use the response type (if defined) to determine whether to process the response as a blob or process as normal.

Notes/things to review

  • I'm following the current TypeScript code that specifies Blob response type will be generated.
  • Axios implementation works in the browser, it does not work when running in Node.JS as it does not support the Blob response type.
  • It is easy to support different response types per client, e.g. Node.JS could be a Stream and fetch could be a Blob. It is more difficult to give the user the option as to what response type they would like.

loganbenjamin avatar Mar 19 '22 01:03 loganbenjamin

Why not add support for bufferArray as well?

yondh avatar Mar 22 '22 08:03 yondh

I'm not actually sure of an easy way to determine between returning a Blob and an ArrayBuffer?

The only way I can think of it is an update to the Service class functions to take an options object. This would also mean more complicated TypeScript to determine the response type.

The reason I chose to use blobs was:

  1. It is currently generated as the TypeScript response type by this project
  2. At a quick glance Blob is supported in more standard functions (e.g. URL.createObjectURL)
  3. You can easily change a Blob to an ArrayBuffer using .arrayBuffer(), e.g. const arrayBuffer = await blob.arrayBuffer() or using the older fileReader.readAsArrayBuffer(blob)

One place which I used as a reference for my decision is: https://github.com/noahunallar/arraybuffer-vs-blob

loganbenjamin avatar Mar 27 '22 05:03 loganbenjamin

In regards to axios - according to docs (https://github.com/axios/axios#request-config):

responseType indicates the type of data that the server will respond with options are: 'arraybuffer', 'document', 'json', 'text', 'stream' browser only: 'blob' responseType: 'json', // default

So if used in Node.js when you set responseType: "blob", "json" will actually be used, which I guess fallbacks to "text" and might result in corrupted data.

yondh avatar Mar 27 '22 10:03 yondh

Ah good spotting, I had tested axios but only on client-side. If running under Node.js I've noticed people opting for streams most of the time when dealing with files. Although I'm not sure how I would know in the case of using axios.

fallbacks to "text" and might result in corrupted data. Yes it would likely do that, this is what also happens currently and the reason for the PR in the first place.

Happy to make changes to the PR, just need some input from the team as to what they would like.

loganbenjamin avatar Mar 29 '22 09:03 loganbenjamin

How will the responseType be determined from the OpenAPI specification?

I was not yet able to get it to generate a responseType: 'blob' entry in the APIRequestOptions of the __request of my route. However, when adding this manually it works like a charm :)

bewee avatar Apr 05 '22 20:04 bewee

Nvm, just figured it out. I'd really love to see this getting merged! Is there anything I could possibly help with?

bewee avatar Apr 05 '22 21:04 bewee

Apologies I didn't have much information in my original PR, I've updated it to better detail how it works.

Nothing to do at the moment just waiting for @ferdikoomen to review it and let me know if there is any changes to be made.

loganbenjamin avatar Apr 06 '22 08:04 loganbenjamin

@loganbenjamin Thanks for the additional info, I will review this asap (bit busy right now, but will try to make some time!). The feature looks very nice from your description and a quick look at the changes.

ferdikoomen avatar Apr 06 '22 19:04 ferdikoomen

Just tried this branch with a newer ionic (6.x) / angular (13.2.x) app, and this works perfectly with the exception of an initial build error:

Error: src/app/_generated/openapi/core/request.ts:209:5 - error TS2322: Type 'Observable<ArrayBuffer>' is not assignable to type 'Observable<HttpResponse<T>>'.
  Type 'ArrayBuffer' is missing the following properties from type 'HttpResponse<T>': body, type, clone, headers, and 4 more.

It appears that changing this line in request.ts:

responseType: options.responseType,

To this:

responseType: options.responseType as any,

alleviates the above error, which appears to be coming from an angular/common/http type def conflict.

Once the above line was changed, Blob responses started coming back as expected with no changes to my openapi spec.

Not sure whether there's a more elegant way to handle this with generics, but hitting that line with the "any" hammer seems to do the trick for now.

jospete avatar Apr 11 '22 02:04 jospete

@jospete thanks for checking it out and responding with detailed information, I've made a small change to fix this.

loganbenjamin avatar May 10 '22 10:05 loganbenjamin

@loganbenjamin I ran into this same issue, and while I troubleshooting and coming up with a similar solution, I also noticed that the [accept] header is always being set to 'application/json', even though the openapi spec for the file download path is "application/octet-stream". This isn't causing me problems, since my server seems to be ignoring the [accept] header in this case-- but this seems like this should be solved in this same PR.

schehlmj avatar May 16 '22 02:05 schehlmj

In an Angular codegen variant, requests are successfully executed (200 OK) but they resolve failed in the underlying HttpClient code, due to their bodies being blobs. In order to make it work fine, responseType: 'blob' is needed in the http.request / http.get call.

maciejgoscinski avatar Jun 03 '22 07:06 maciejgoscinski

In an Angular codegen variant, requests are successfully executed (200 OK) but they resolve failed in the underlying HttpClient code, due to their bodies being blobs. In order to make it work fine, responseType: 'blob' is needed in the http.request / http.get call.

Hi @maciejgoscinski, thank you for pointing this out. we are facing the same problem using Angular

helloworld121 avatar Aug 08 '22 11:08 helloworld121

Hi, what is status of this issue? I have the same problem with blob request from api

kkulebaev avatar Oct 20 '22 14:10 kkulebaev

Any news on this?

insulationman avatar Oct 31 '22 13:10 insulationman