-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update dependency got to v9 #10861
Update dependency got to v9 #10861
Conversation
1e6edd5
to
75f0308
Compare
@kevinansfield the change in retry logic has some implications on the amount of external request Ghost does. It will now increase when 500 is returned due to 2 additional retries (described in commits above). Don't necessarily see any bad implications out of it but wanted to get a second opinion on this. If we want to avoid this behavior, there's an option to override the retries with status codes used in got v8.x |
@@ -133,7 +133,8 @@ describe('Request', function () { | |||
|
|||
const requestMock = nock('http://nofilehere.com') | |||
.get('/files/test.txt') | |||
.reply(500, {message: 'something aweful happend', code: 'AWFUL_ERROR'}); | |||
.times(3) // 1 original request + 2 default retries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gargol Was trying to understand from changelog/commit, is the times
here with the value representing default, which is it will 3 times before returning 500? If so, is it possible to override it to use 1
instead of 3
before returning the status code? Also, any reason we skipped sending the message along with status code below, as in old code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is configuring nock (mocking external server that is pinged by got
), so it had to adjust to the changes to the new default behavior of got (now it makes 2 additional retries when 500, and bunch others, is returned). If we just return 500 after 1st request, got will not be able to reach anything on the retry.
Think the question is more if we should allow making 2 additional requests for your wrapper of got.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, got it. Yeah i don't see any bad implication either, and seems to be a sane default. Either ways, we should still be adding the message along with status code here in tests like previously, unless i am missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rishabhgrg have added the response body back to the test and also a check in the test itself. Merging it 🚢
b92f35e
to
1dbeb8a
Compare
@gargol I had clicked the checkbox to rebase after merging other renvoate PRs, but that apparently removed your commit with the fix, sorry! 😓 That's whats causing the tests to fail again as well, do you still have the commit locally? |
- The underlying issue is change in retry behavior in 'got' (sindresorhus/got@a3e77de) - Now 500 responses trigger 2 default retries
- As mentioned in: https://github.com/sindresorhus/got/releases/v9.0.0
PR has been edited👷 This PR has received other commits, so Renovate will stop updating it to avoid conflicts or other problems. If you wish to abandon your changes and have Renovate start over you may click the "rebase" checkbox in the PR body/description. |
- Also added check for it
This PR contains the following updates:
8.3.2
->9.6.0
Release Notes
sindresorhus/got
v9.6.0
Compare Source
init
hook (#683)677d0a4
beforeError
hook (#696)29ffb44
v9.5.1
Compare Source
203dadc
d136e61
d136e61
v9.5.0
Compare Source
5d20a43
a1eadfe
v9.4.0
Compare Source
9f3a099
33b838f
socket
timeout on error.c8e358f
v9.3.2
Compare Source
d8dd881
07b3ce5
v9.3.1
Compare Source
url
argument when it's an object.191e00a
content-length
header when upload body size isnull
.311b184
v9.3.0
Compare Source
b392f60
beforeRedirect
,beforeRetry
, andafterResponse
hooks.325409c
fbaaa2a
body
property inHTTPError
.fdc0fa6
a07b2be
25f18be
v9.2.2
Compare Source
Location
redirect URLs. (#605)7ae6939
3ad3950
got.extend()
. (#608)292f78a
v9.2.1
Compare Source
b8480f3
5f191b9
v9.2.0
Compare Source
8191daf
cookieJar
option. (#596)eedebc9
78a56ec
content-length
header to zero when doing a PUT request with no body. (#584)9a966ec
query
option when it's a string. (#589)f23f350
response
object. (#592)267cb66
HTTP trailers are not supported
error. (#598)e66a6b6
v9.1.0
Compare Source
got.mergeInstances()
which lets you easily compose multiple instances of Got into a single one. It's like plugins without any of the plugin mess. (#510)f0b190a
baseUrl
option. (#579)c901c46
3d98b9b
query
option to be aURLSearchParams
instance. (#565)b8a086f
a3e77de
get-stream
to 0.4.0 for improved handling of stream edge-cases. (#578)45d3a60
method
option. (#547)ecf3180
agent
option withgot.extend()
. (#577)2ffcd49
got.stream()
. (#550)ae5b114
eb652f8
v9.0.0
Compare Source
Got version 9 is a massive release! Many new awesome features and we have pretty much fixed all the open issues regarding Got bugs on the issue tracker.
You might ask, why not follow the Node.js LTS cycle and target Node.js 6. In short, async/await and WHATWG URL, which enabled us to simplify the codebase and reduce the dependency-tree considerably. Got v8 is a stable release, and you can continue using that if you need to support Node.js 6.
98b5664
The option was renamed from
retries
toretry
and it now accepts either a number of retries or an object with the ability to specify the number of retries, HTTP status codes and methods to retry on, and a function to decide how to retry. See the docs for more.Migration:
-
{ retries: 4 }
→{ retry: 4 }
-
{ retries: () => { … } }
→{ retry: { retries: () => { … } } }
.canceled
property to.isCanceled
.00fdeea
body
option being an Array whenform: true
.dfe5b1c
The built-in
new URLSearchParams()
API doesn't support this either and it's a weird use-case. If you need support for this, just don't setform: true
and handle stringifying yourself..pipe()
was called after the response has been emitted.This makes us sure you receive whole response in case flowing mode is on (#223).
Improvements
- Got v9:
- Got v8:
- Request (latest):
timeout
option was rewritten to be more reliable and flexible. You can now set a timeout for every phase of the request if you want, or just for the whole request. That's up to you.da4f236
got.extend()
which lets you easily create instances of Got with some options overridden.bc41a49
got.create()
which is a more advanced and powerful version ofgot.extend()
. With this API, you can create your own niche-specific instance of Got, for example, an HTTP-client for GitHub.bc41a49
beforeRequest
hook.107756f
request
andresponse
events to the Promise API.e86aad7
content-length
header is now also automatically set if thebody
option is set to afs.createReadStream
instance.6e7a455
user-agent
header by passing in'user-agent': null
as a header.e473a26
body
option can now be any kind of object, not just a plain object.7a49ce7
Bug fixes
query
option.07a91cc
f621184
got.stream()
.83bc44c
Other
b54b680
Team
Welcome @szmarczak and @brandon93s as maintainers 🎉
Special shoutout to @jstewmon for helping us with many of the above improvements and fixes 🙌
All changes
Renovate configuration
📅 Schedule: "before 3am on Tuesday" (UTC).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻️ Rebasing: Whenever PR becomes conflicted, or if you modify the PR title to begin with "
rebase!
".🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR has been generated by Renovate Bot. View repository job log here.