[REQ] Typescript-fetch returns error message
Is your feature request related to a problem? Please describe.
I have use cases where I am needed to get a more specific error code than the basics HTTP codes. For example : I have several cases when I can get a 403. End of subscription or banned.
Describe the solution you'd like
I want to get the API's message returning to my request and not only the HTTP code and "Response returned an error code" as actualy happening.
To resolve this issue I changed the code of the request method in the 'runtime.ts' file as followed :
const message: string = !response.ok ? (await response.json()).message : 'Response returned an error code'; throw new ResponseError(response, message);
I second this request! Not having access to error messages returned by the server via an OpenAPI generated TypeScript client is a significant pain point.
You should have access to error messages via the ResponseError :
try {
return api.getPetById({petId: 1})
} catch (error: unknown) {
let message = 'An error occurred'
if (error instanceof ResponseError) {
message = (await error.response.json()).message;
}
// Do something with the error
}
I have a PR here where I attach the http response to ResponseError. It's currently being passed but it doesn't do anything with it, the response is just dropped. With this approach, when the error is thrown, the user may do error.response to access the response object. I'll polish it (tests) but I wanted to have your feedback first since I stumbled upon this thread as I was looking for a solution to the same problem as you are facing. Let me know! :)
How is this being handled? The PR was closed, but I would be happy if you could respond!
How is this being handled? The PR was closed, but I would be happy if you could respond!
What @Halu89 mentioned is the way to go. The response is already attached to the error object, you just need to ensure it's of the proper type to access the response, per their example.
Ran into similar confusion about how this thing works, it's tricky because it doesn't render to anything if you try to send it forward while debugging. What you do need to know is that e.response in this case is a Response object, and can be interacted with as such.
@Halu89 's comment does work, but I would prefer not to wrap all my api calls with a try catch module. What I would prefer is that for instance all the ...raw methods would just return the response no matter the status code. Or if this would be breaking or too invasive, maybe generate a third method per endpoint that will always return the response no matter the status code. For testing purposes this would be a great addition.
Yeah this was an incredibly poor design decision and screwed up all of the types within typescript. In our case, we were returning an AxiosPromise<APIResponse> from the API-generated code, but because of the ridiculous try/catch blocks (FYI returning a 400 or 500 response from an API is a valid scenario, not an exception) it conflates the AxiosResponse with the ResponseError.response. Which is both not a valid approach in any real OOP language and messes up all of the type-hints.
This approach is actually a best practice in TypeScript. When calling api.getPetById({ petId: 1 }) (from @Halu89's example), TypeScript can't know in advance what kind of error might be thrown. It could be anything—for example, fetch itself throws TypeError in various cases, such as when the network is down (MDN docs). In that case, the error wouldn’t be a ResponseError because no request was made at all. Or, if there's a bug in the client code, an InternalError could be thrown.
Since errors can come from many sources, TypeScript encourages treating them as unknown and then using instanceof or type guards to narrow them down:
try {
api.getPetById({ petId: 1 });
} catch (error: unknown) {
// Handle error appropriately
}
I get that wrapping calls in try/catch can feel tedious, but it helps prevent unexpected crashes. Could a utility function help reduce the repetition?
This approach is actually a best practice in TypeScript. When calling
api.getPetById({ petId: 1 })(from @Halu89's example), TypeScript can't know in advance what kind of error might be thrown. It could be anything—for example,fetchitself throwsTypeErrorin various cases, such as when the network is down (MDN docs). In that case, the error wouldn’t be aResponseErrorbecause no request was made at all. Or, if there's a bug in the client code, anInternalErrorcould be thrown.Since errors can come from many sources, TypeScript encourages treating them as
unknownand then usinginstanceofor type guards to narrow them down:try { api.getPetById({ petId: 1 }); } catch (error: unknown) { // Handle error appropriately } I get that wrapping calls in
try/catchcan feel tedious, but it helps prevent unexpected crashes. Could a utility function help reduce the repetition?
The MDN reference you listed makes sense for exceptions. Those don't include status code responses from valid requests. It would make perfect sense to throw an exception if the headers were invalid or a GET request had a payload. In fact, in the MDN document you referenced, their first example shows the response coming back and if it is not "okay" (!response.ok), then the developer throws an exception, not the fetch method. I would expect that I - as the developer - would be responsible for handling non-200 responses, not the openapi-generator framework. Besides, there are valid reasons that a backend would return a non-200 error, such as 401 Unauthorized error returned if the bearer token has expired. The best practice in Typescript would be to return a Response object and allow the developer to throw an exception or handle the Response correctly and throw an Exception for the reasons listed under the section titled "Exceptions". In fact, that's what MDN says in the third paragraph of fetch: "A fetch() promise does not reject if the server responds with HTTP status codes that indicate errors (404, 504, etc.). Instead, a then() handler must check the Response.ok and/or Response.status properties."
I see your point. So there is a preference in always returning the response object if there is one, instead of throwing regardless of the status code.
As for the messed up type hints, I assume you are referring to the fact that the response, in case of an error, is buried/less discoverable under the wider type unknown and it's up to the developer to narrow down the type to access the underlying response object, is that right or were you referring to something else?
It's a design choice indeed. The library authors have provided ways to customize those choices via generator options. Could a new option be introduced to customize the behavior? (If it doesn't already exist, I didn't check).
As for the original topic from @AlexandreFinne, the suggestion is to return the error message from the response error. The suggestion assumes that 1) the error response is JSON content and 2) a message property is available within the deserialized JSON response. Is that too opinionated? What if the response doesn't come from the app server, but from a load balancer that may have thrown a 502 Bad Gateway?
As for the original topic from @AlexandreFinne, the suggestion is to return the error message from the response error. The suggestion assumes that 1) the error response is JSON content and 2) a
messageproperty is available within the deserialized JSON response. Is that too opinionated? What if the response doesn't come from the app server, but from a load balancer that may have thrown a 502 Bad Gateway?
Yeah that's a good point, @aconrad . That's why I think it's preferable to just let the developer handle those responses, regardless of what content type or status code range is returned. The generated client-side code should never assume how the client/server interactions occur. In our case, a lot of the problem is that I set the response variable from the openAPI call to the ResponseError.response to avoid having to switch on the status code twice (once within the catch block for the ResponsError and once for valid responses returned from the API call that I need to handle on the client-side). I could just assume that all ResponseErrors only contain 400 and 500 errors and have the two switch statements but it seems less intuitive from a maintenance perspective and redundant - it just adds more switch code and default cases.
I like your suggestion of creating a new generator option to return responses regardless of the status code. I think that would acquiesce existing implementations that rely on the current "exception throw" paradigm but would allow new projects in the future to handle their own status codes independently.