Conversation
smithamax
changed the title
v2.0.0-rc.3, to see the old v2.0.0-rc.2 look or the tag or other/v2.0.0-rc.2 branch
| - Options are now native `fetch` options not `got` options | ||
| - Errors are now native `fetch` errors not `got` errors | ||
| - default `_handlerResponse` now returns the parsed body of the response | ||
| - Timout is no longer an option, pass a `signal` on the fetch options instead |
There was a problem hiding this comment.
Worth having an example of how to do this?
There was a problem hiding this comment.
Yeah probably, thinking of adding context support in the future so you can do it via that too
| - Timout is no longer an option, pass a `signal` on the fetch options instead | ||
| - No longer supports the `retry` option, wrap your call to `request` in a retry | ||
| loop instead | ||
| - No longer supports the `http_client_request_stage_duration_seconds` metric |
There was a problem hiding this comment.
What did this metric provide?
There was a problem hiding this comment.
It's pretty nice, it shows what http spent time on, see the last graph here
But fetch doesn't give us the individual timings so can't do it
There was a problem hiding this comment.
i don't think ive ever scrolled down to see that graph
| - Errors are now native `fetch` errors not `got` errors | ||
| - default `_handlerResponse` now returns the parsed body of the response | ||
| - Timout is no longer an option, pass a `signal` on the fetch options instead | ||
| - No longer supports the `retry` option, wrap your call to `request` in a retry |
There was a problem hiding this comment.
Not worth building this in to the lib? Or having a best practices example?
There was a problem hiding this comment.
I don't think we use it enough to bother building it in, and it's pretty easy for the caller to just wrap in a retry loop.
If we build it in we have to add logic around response codes and method types in a generic way. The caller can just figure out this themself
|
|
||
| - The Response object is now from native `fetch` and not `got` | ||
| - Options are now native `fetch` options not `got` options | ||
| - Errors are now native `fetch` errors not `got` errors |
There was a problem hiding this comment.
Are there some common error cases we should document?
There was a problem hiding this comment.
Looking the deliverect PR I think a big one would be statusCode -> status
There was a problem hiding this comment.
and no more .body or .headers, use .response instead
There was a problem hiding this comment.
Mostly because all the error conditions are harder to hit when testing
There was a problem hiding this comment.
I think the example _handlerError does a pretty good job showing what the basic pattern should be
2 participants
I've re-done v2 and published as
v2.0.0-rc.3, to see the oldv2.0.0-rc.2look or the tag or other/v2.0.0-rc.2 branchrc.2 seemed a little overbuilt when it came to errors etc. Has a lot going on for metrics
to try and make
http_client_request_stage_duration_secondswork, but didn't really do anything.On top of all that, it didn't even remove
gotfrom package.jsonSo I started again with a simpler approach
The main breaking change is that we switched from
gotto native nodefetchfor the underlying HTTP client. This means that the API is now more aligned with
the native fetch API.
fetchand notgotfetchoptions notgotoptionsfetcherrors notgoterrors_handlerResponsenow returns the parsed body of the responsesignalon the fetch options insteadretryoption, wrap your call torequestin a retry loop insteadhttp_client_request_stage_duration_secondsmetricAny project upgrading will have to to a fair bit of work, but it'll mostly be in the
_handlerResponseand_handlerErrorfunctions