-
Notifications
You must be signed in to change notification settings - Fork 365
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
refactor: replace got with node-fetch on dev-miscellaneous.test.js #6235
Merged
kodiakhq
merged 35 commits into
netlify:main
from
hereje:refactor/replace-got/dev-miscellaneous.test.js
May 17, 2024
Merged
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
4248090
refactor: replace got with node-fetch on dev-miscellaneous.test.js
hereje dd3a820
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje ff37898
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 12a17de
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje c3b6577
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 7f7f51b
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 8460a4f
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje cd52475
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje 54c995d
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
sarahetter ec7042e
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje 3f3b1a8
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
sarahetter d5d3ba7
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 16c44f1
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje aed1570
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje 88b429c
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 3c740fe
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 48de871
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje 6df4f23
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 112b208
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje 34263b5
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 1998f17
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 3af26ca
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje cc7e0d3
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje b2dae41
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 5777c9e
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 2d27359
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje 7a4a77b
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje f27672a
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje d418f20
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
sarahetter c5ef5b2
Merge branch 'netlify:main' into refactor/replace-got/dev-miscellaneo…
hereje ba00765
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
sarahetter 9e0cf9c
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
hereje c18d231
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
mrstork 5a2f690
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
mrstork b610220
Merge branch 'main' into refactor/replace-got/dev-miscellaneous.test.js
kodiakhq[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I played around with removing this delay and was able to uncover something potentially useful, but did not ultimately uncover the root of the problem. With no pause, we will see different results:
mac
+node v20.12.2
+got
->'Hello world'
windows
+node v20.12.2
+got
->'Hello world'
mac
+node v20.12.2
+fetch
->'Hello world'
windows
+node v20.12.2
+fetch
->ECONNREFUSED
I also found this potentially related node 18 issue node-fetch/node-fetch#1624. Even though the test is failing on a different version of node, I tried changing the url in the test from
localhost
to127.0.0.1
but saw no change in behaviour.It doesn't actually solve the underlying issue, but given the above, increasing this delay up from
500
should get the tests to pass.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.
To be clear, the changes in this PR do not introduce an issue (as they are just modifying a test), they simply bring to light an issue that exists somewhere in our codebase. Some interaction between some or all of the following components:
edge proxy
,fetch
,builder
(on windows)