Conversation
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
| case 404: | ||
| throw NotFoundError; | ||
| case 409: | ||
| throw new ConflictError(response.statusText || 'Conflict'); |
There was a problem hiding this comment.
Can rename this method to something like assertNotClientErrorResponse? Also, callers may want the error to have more details, such as request status, method, and url. And zooming out, is it appropriate to always throw on these response types? Why not return the response and let the caller sort it out? Keep in mind that JS/TS has horrible exception matching support, so its often much more ergonomic to inspect the response object than perform duck-typing on an opaque error object of type unknown
There was a problem hiding this comment.
Can rename this method to something like assertNotClientErrorResponse?
this is internal function only to manage response error, name doesn't matter
Also, callers may want the error to have more details, such as request status, method, and url.
Why not return the response and let the caller sort it out?
SDK is there to hide the HTTP details, so I would expect they don't need to and don't want them to have to
- What I did
detect standard HTTP status and convert into SDK error types
- A picture of a cute animal (not mandatory but encouraged)