refactor: use openapi-fetch #47
Conversation
src/common/atlas/apiClient.ts
Outdated
| private errorMiddleware = (): Middleware => ({ | ||
| async onResponse({ response }) { | ||
| if (!response.ok) { | ||
| throw new ApiClientError(`Error calling Atlas API: ${await response.text()}`, response); |
There was a problem hiding this comment.
are we guaranteed the response text? is there a chance this could error as well?
src/common/atlas/apiClient.ts
Outdated
| if (!response.ok) { | ||
| try { | ||
| const text = await response.text(); | ||
| throw new ApiClientError( |
There was a problem hiding this comment.
this does mean that what we'll get is
Error calling Atlas API: Error calling Atlas API:...
There was a problem hiding this comment.
More like Error calling Atlas API: [400 Bad Request] Invalid json or some other text from backend
There was a problem hiding this comment.
I think passing the response object is sufficient, we can just omit the response text
There was a problem hiding this comment.
What do you mean? response is a property and does not display error message directly. You prefer to hide error message?
There was a problem hiding this comment.
I'd say so. If we want to include properties of the response inside the message (including in format [${response.status} ${response.statusText}]) then let's do it inside the ApiClientError class's super call to message so it's standardized and enforced. And in that sense avoid anything async.
fixes #33