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

[loganbenjamin]

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.

[yondh]

Why not add support for bufferArray as well?

[loganbenjamin]

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

[yondh]

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.

[loganbenjamin]

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.

[bewee]

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]

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

[loganbenjamin]

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.

[ferdikoomen]

@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.

[jospete]

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.

[loganbenjamin]

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

[schehlmj]

@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.

[bertramr]

Suggested change

	{{~#equals results.length 1~}}
	{{~#equals results.0.base 'binary'~}}
	responseType: 'blob',
	{{/equals}}
	{{~/equals~}}
	{{~#equals results.0.base 'binary'~}}
	responseType: 'blob',
	{{/equals}}

I'd like to suggest to not check on results.length 1 as there might be multiple valid responses, e.g. some 202 not ready response, which is not a blob and thus would be filtered out.

However only checking for the first element seems fine to be, as the 200 success response will be sorted and should always be the first element

[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.

[helloworld121]

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

[kkulebaev]

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

[insulationman]

Any news on this?

[theo-dolle]

@bertramr i have the same issue in angular. Do you need more context ? 🙏

[bertramr]

@bertramr i have the same issue in angular. Do you need more context ? 🙏

Hello there,

I wanted to share with you a pull request that I think you might find helpful: #1043

Please note that this pull request changes the original design and, as mentioned in the description, it's unlikely to be included in the main repository. However, if you're interested in using it, you can maintain your own version.

I hope you find this helpful!

[aamerSunwapta]

Hey any updates on when this can be merged?

[waza-ari]

Would love to see this too, would be quite nice to get this merged. We have a similar requirement where we get binary data but currently we get the .text() response.

[ricardopinto]

Looking forward for this merge too. @ferdikoomen how else can we help to expedite this?

[JeroenvdBurg]

@ferdikoomen LGTM

[aurelien-brabant]

We need it also, would be super helpful to have this merged :)

[joZephhh]

Any updates on this ? Thanks! 🙏

[rinkaaan]

Hello, any updates?

[xdubx]

@loganbenjamin can you fix the conflicts and can some from the authors review the change?

[benny-noumena]

It would be so nice to merge this PR. Any reason it is not by now?

[jordanshatford]

@benny-noumena check out our fork @hey-api/openapi-ts which has added more support for blob responses. It may fix this issue for you, and if not please open and issue there and I will work to fix it.