Skip to content
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

chore(deps): bump got from 11.8.3 to 12.0.0 #7370

Merged
merged 14 commits into from
Jan 29, 2022

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Dec 10, 2021

Bumps got from 11.8.3 to 12.0.0.

Release notes

Sourced from got's releases.

v12.0.0

Introducing Got v12.0.0 🎉

Long time no see! The latest Got version (v11.8.2) was released just in February ❄️ We have been working hard on squashing bugs and improving overall experience.

If you find Got useful, you might want to sponsor the Got maintainers.

This package is now pure ESM

Please read this. Also see sindresorhus/got#1789.

Required Node.js >=14

While working with streams, we encountered more Node.js bugs that needed workarounds. In order to keep our code clean, we had to drop Node.js v12 as the code would get more messy. We strongly recommend that you update Node.js to v14 LTS.

HTTP/2 support

Every Node.js release, the native http2 module gets more stable. Unfortunately there are still some issues on the Node.js side, so we decided to keep HTTP/2 disabled for now. We may enable it by default in Got v13. It is still possible to turn it on via the http2 option.

To run HTTP/2 requests, it is required to use Node.js v15.10 or above.

Bug fixes

Woah, we possibly couldn't make a release if we didn't fix some bugs!

  • Do not throw on custom stack traces (#1491) 49c16ee54fb19ea7aa77e24ac8c2b602f0aad265
  • Remove automatic content-length on ReadStream (#1510) 472b8ef9d9fc7713b740981a8b1103a7a9111b26
  • Fix promise shortcuts in case of error status code (#1543) ff918fb6dedb6d8b23421497ec890d43f45121b7 1107cc625e4cc469276483316c48896a21f6251a
  • Invert the methodRewriting option 51d88a0efed56760d116c5b911cea71e3265c787
  • Fix url not being reused on retry in rare case (#1487) 462bc630015064fa4ad4358cf28d24f95e1c958b
  • Fix hanging promise on HTTP/2 timeout (#1492) a59fac415ac013a48b1d514837628a5cf81d6878
  • Prevent uncaught ParseErrors on initial successful response (#1527) 77df9c33db5ba3126f54317171e1cfcfceefc3d5
  • Throw an error when retrying with consumed body (#1507) 62305d77d3428b5c714d21b4bbee68cc75b5f787
  • Fix a Node.js 16 bug that hangs Got streams 06a2d3d7d8d4fcc6898b6364d1a18ca1d407092b
  • Fix default pagination handling for empty Link header (#1768) 1e1e50647e93d038a4cc6a9bbbfbf61165d8fd39
  • Fix incorrect response.complete when using cache 9e15d887da3b065940bbc8ca38f9c748a0bbc75e
  • Fix Cannot call end error when request returns a Writable 226cc3995f6e16938163ebde24d8762e7dcd15e2
  • Fix Request options not being reused on retry 3c23eea5a096f6f8ea0edf3e2a27e1caca88acf9
  • Fix types being not compatible with CommonJS 3c23eea5a096f6f8ea0edf3e2a27e1caca88acf9
  • Fix got.paginate does not call init hooks (#1574) 3c23eea5a096f6f8ea0edf3e2a27e1caca88acf9
  • Generate a new object when passing options to the native https module (#1567) 3c23eea5a096f6f8ea0edf3e2a27e1caca88acf9
  • Remove stream reuse check (#1803) 9ecc5ee76f77aafd5100520d9d8789c491c8fb24
  • Fix merging searchParams (#1814) 1018c2029eea1f5b75b5120265996f1c0b3c12ae 732e9bd9406ba1c3dd64b445264e891f33fc0254
  • Fix unhandled exception when lookup returns invalid IP early (#1737) 2453e5e4213fe036a0108de3e4db414dcf2b4c30
  • Fix relative URLs when paginating 439fb82d2a07cece417a18c47e37cfdeaaf38db7

... (truncated)

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added the dependencies Related to dependency updates label Dec 10, 2021
@shields-cd shields-cd temporarily deployed to shields-staging-pr-7370 December 10, 2021 18:16 Inactive
Bumps [got](https://github.com/sindresorhus/got) from 11.8.3 to 12.0.0.
- [Release notes](https://github.com/sindresorhus/got/releases)
- [Commits](sindresorhus/got@v11.8.3...v12.0.0)

---
updated-dependencies:
- dependency-name: got
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/got-12.0.0 branch from 4e70e9c to 89d69e2 Compare December 10, 2021 18:22
@shields-cd shields-cd temporarily deployed to shields-staging-pr-7370 December 10, 2021 18:23 Inactive
@chris48s
Copy link
Member

Haven't had time over the weekend, but I will try and have a look at updating to account for the breaking changes in this release sometime next week.

@chris48s chris48s temporarily deployed to shields-staging-pr-7370 December 14, 2021 20:43 Inactive
@chris48s
Copy link
Member

Made a start on this but I still have some issues to sort out. Most of the test failures are mocked tests. There is some strange issue at the intersection of got and nock that I haven't got to the bottom of yet

@chris48s chris48s temporarily deployed to shields-staging-pr-7370 December 18, 2021 19:26 Inactive
@chris48s chris48s temporarily deployed to shields-staging-pr-7370 December 18, 2021 19:31 Inactive
@chris48s chris48s force-pushed the dependabot/npm_and_yarn/got-12.0.0 branch from 1683031 to 7d4e9dc Compare December 18, 2021 19:40
@chris48s chris48s temporarily deployed to shields-staging-pr-7370 December 18, 2021 19:41 Inactive
@chris48s chris48s temporarily deployed to shields-staging-pr-7370 December 18, 2021 19:45 Inactive
@chris48s
Copy link
Member

Right 178d35b gets the core test suite passing
Next lets start running the service tests

@chris48s chris48s changed the title chore(deps): bump got from 11.8.3 to 12.0.0 chore(deps): bump got from 11.8.3 to 12.0.0; run [*****] Dec 18, 2021
@chris48s
Copy link
Member

Right then
At this point, the actual application is working fine. I think the remaining issues are just about the interaction between got and nock under test.

If you look at
https://app.circleci.com/pipelines/github/badges/shields/10514/workflows/a134e8aa-d199-4c84-b247-eb7d1172ff9d/jobs/167589
the first nock-based test works. Then every subsequent nock test fails. So any given nock-based test runs in isolation, but then if you pick any two the second one fails.

Doing this: 178d35b seems to fix it. Doing that gives us a passing run for the core tests:
https://app.circleci.com/pipelines/github/badges/shields/10515/workflows/ff321837-b15d-4296-a2bf-e10b66c26c3f/jobs/167594

I'm not totally sure what the original intent of that was.

Anyway, next step is the service tests. If you run the service tests, all the mocked tests fail (even the first one):
https://app.circleci.com/pipelines/github/badges/shields/10516/workflows/546abac4-a044-473f-b7d9-fd6155ef5f2c/jobs/167602

(I've tried to structure the commit/PR history so you can see this journey)

I guess we're looking at something in the setup for when we .intercept() in the service tests

but I think I want to understand more about why 178d35b fixes the core tests first.

Hmmm 🤔

@calebcartwright
Copy link
Member

Whew, this one is sounding pretty gnarly

@chris48s
Copy link
Member

this one is sounding pretty gnarly

Yes and no. The actual changes needed to get the application working with the new version are quite small and mostly called out in the release notes. The gnarly bits are all to do with mocking under test (although working out which tests were failing for what reasons did take some time). While working through this I have been looking out for any issues on the got or nock repos to see if anyone else is hitting similar issues. One thing to note is:

Given this, I might keep an eye on the nock repo and see what happens when they bump got in their own test suite in a week or two. It might give us some pointers..

@chris48s
Copy link
Member

chris48s commented Jan 2, 2022

Am now subscribed to nock/nock#2274
Worth noting I tried upgrading another much smaller/simpler project to got 12 which also relies on nock for mocking and hit similar issues.

@chris48s
Copy link
Member

Looks like there is an upstream fix for this
sindresorhus/got#1959
Not tested yet, but I'll circle back to this and update

@chris48s
Copy link
Member

Nearly there but I've run out of time this evening. I'll have another look over the failing service tests again tomorrow. I think most of them are the usual suspects but there might be one or two more sneaky ones like OPM hiding in there.

@calebcartwright
Copy link
Member

Nearly there but I've run out of time this evening. I'll have another look over the failing service tests again tomorrow. I think most of them are the usual suspects but there might be one or two more sneaky ones like OPM hiding in there.

Excellent news. I know @PyvesB fixed a few sets of tests that were just merged so a rebase/merge should reduce the surface of tests that need to be inspected

@chris48s
Copy link
Member

Ah yes. Thanks for pointing that out 💡 I was comparing the test runs on this branch against the latest ones on daily-tests and wondering why I had some many more failing tests. Of course they were all the ones we merged fixes for the other day. I did see those PRs but just hadn't put two and two together I needed to update my branch :)
I think this should be good to go now.

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Jan 14, 2022

A newer version of got exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@chris48s
Copy link
Member

Anyone fancy giving this a quick review? I didn't just go ahead and merge it as there are some non-trivial code changes to account for the update but it appears to have fallen through the cracks.

@calebcartwright
Copy link
Member

Anyone fancy giving this a quick review? I didn't just go ahead and merge it as there are some non-trivial code changes to account for the update but it appears to have fallen through the cracks.

Indeed, will take a look shortly

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@chris48s chris48s changed the title chore(deps): bump got from 11.8.3 to 12.0.0; run [*****] chore(deps): bump got from 11.8.3 to 12.0.0 Jan 29, 2022
@chris48s chris48s merged commit 0f288a4 into master Jan 29, 2022
@chris48s chris48s deleted the dependabot/npm_and_yarn/got-12.0.0 branch January 29, 2022 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Related to dependency updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants