Got: v11 beta feedback

[sindresorhus]

We just published the first beta for Got v11. Try it out and let us know: https://github.com/sindresorhus/got/releases/tag/v11.0.0-beta.1

[dylang]

Nice work!

Only spent a few minutes with it and already excited that we are able to provide typed mocks in Jest similar to v9. I couldn't figure out how to cleanly do it in v10.

_example using v11.0.0-beta.1_

mocked(got.get).mockReturnValue({
  json: jest.fn().mockResolvedValue([
    {
      id: 8982,
      name: 'Ready for Review'
    }
  ])
});

[simontabor]

Just updated like-for-like and a couple of notes:

Experimental APIs

Resolved by updating to later version of Node.js 10

Looks like you're using experimental APIs in Node.js 10 - this will be quite annoying seeing as Node.js 10 is in LTS for another year. Using jest, this is logged many times by my test suite

(node:54054) ExperimentalWarning: The dns.promises API is experimental
(node:54054) ExperimentalWarning: The fs.promises API is experimental

Nock

My nock tests have started failing, haven't had time to look into why

(node:54054) UnhandledPromiseRejectionWarning: Error: premature close
(node:54054) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:54054) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:54054) UnhandledPromiseRejectionWarning: Error: premature close
(node:54054) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)
(node:54054) UnhandledPromiseRejectionWarning: Error: premature close
(node:54054) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 6)
events.js:180
    throw err; // Unhandled 'error' event
    ^

Error [ERR_UNHANDLED_ERROR]: Unhandled error. (Error: socket hang up)
    at OverriddenClientRequest.emit (events.js:178:17)
    at process.nextTick (/Users/simon.tabor/Development/fe-router/node_modules/nock/lib/intercepted_request_router.js:108:11)
    at process._tickCallback (internal/process/next_tick.js:61:11)

Cached responses return 304 status code

Not sure if this is intentional, it might be useful to see that the server responded with 304, but exposing it to the clients will probably break a lot of implementations. Nice to see that cached responses now have response.timings though, before the timings for 304 responses were missing.

Fixed GZIP cached response issue

1158, thanks! When do you expect v11 to be out of beta?

[sindresorhus]

When do you expect v11 to be out of beta?

When we've fixed the biggest blocker issues that turn up here. It's already feature complete.

[szmarczak]

Experimental APIs

Update your Node.js 10 version

[simontabor]

Experimental APIs

Update your Node.js 10 version

My only concern would have been AWS Lambda, which appears to be using Node.js 10.19.0 with stable support, so can consider that a non-issue

[szmarczak]

Nice to see that cached responses now have response.timings though, before the timings for 304 responses were missing.

Ummm, cached responses don't have timings... Can you reproduce the issue in a RunKit example?

[szmarczak]

My nock tests have started failing, haven't had time to look into why

The premature close are stream.pipeline(..) errors. The beta doesn't use pipeline anymore as there's no need to.

[simontabor]

Nice to see that cached responses now have response.timings though, before the timings for 304 responses were missing.

Ummm, cached responses don't have timings... Can you reproduce the issue in a RunKit example?

I mean cached but with a revalidation, so a 304 response from the server. Before, these had no timings, despite making an actual request to the remote server

[simontabor]

My nock tests have started failing, haven't had time to look into why

The premature close are stream.pipeline(..) errors. The beta doesn't use pipeline anymore as there's no need to.

Is there a fix? I have no custom code for stream.pipeline

[szmarczak]

I have no custom code for stream.pipeline

Not sure how to interpret this sentence. Are you saying that you are using stream.pipeline or are you saying that you have not altered with the Node.js codebase?

[simontabor]

I have no custom code for stream.pipeline

Not sure how to interpret this sentence. Are you saying that you are using stream.pipeline or are you saying that you have not altered with the Node.js codebase?

I'm saying that I'm using a very basic implementation of got and nock (and of course haven't altered the Node.js codebase), so I'd expect there to be no errors

[szmarczak]

So you are not using stream.pipeline then?

[szmarczak]

The example gives no errors. It's perfectly valid.

[simontabor]

Sorry the example was slightly wrong.

Here's got v11: https://runkit.com/simontabor/got-nock-v11/1.0.0
vs got v10: https://runkit.com/simontabor/got-nock-v11/2.0.0

In v10, you get the proper error (no matched request), in v11, i get socket hang up

[szmarczak]

So you are not using stream.pipeline then?

You still haven't answered the question... :P But that example looks very promising, thanks for submitting!

[szmarczak]

In v10, you get the proper error (no matched request), in v11, i get socket hang up

If the request errors, the request is aborted. It's just that nock implemented the request.abort() function improperly.

[simontabor]

haha sorry, doing too many things at once - no I'm not using stream.pipeline.

Sounds like it's an issue with nock then, and has become noticeable due to the underlying changes in v11?

[szmarczak]

If you're talking about the premature close errors, I need a reproducible example.

The socket hang up errors are Nock-related 10000%.

[szmarczak]

@simontabor Your example made me discover another Node.js bug located in its core. If you request.abort() on a successful response, the socket is destroyed although it should not. Thanks.

[szmarczak]

nodejs/node#32851

If you do stream.pipeline(got.stream('https://google.com'), new stream.PassThrough()) the socket will be destroyed. This is unacceptable as it breaks the keepAlive functionality in http Agents.

[simontabor]

Temporarily commenting out the this[kRequest].abort(); calls in Request._destroy stops the premature close errors too, so it's all part of the same issue

[szmarczak]

I made a workaround for the Node.js issue: c15e020286239ef2d458fd836298e5cb24bd2a13

[adityapatadia]

I would still love to be able to specify a replacement for cacheable-request module. I have been saying it from quite some time because that one module is extremely useful but full of bugs. If that is not possible, I expect @szmarczak to upgrade that module the way cacheable-lookup was done.

[szmarczak]

It's on the list: #875 Once Got 11 is released, I'll start to work on the enhancement issues and update our dependencies.

[adityapatadia]

Good to know. As I said before, let us know if you need any help in coding.
On 19 Apr 2020, 3:47 PM +0530, Szymon Marczak [e-mail], wrote:

It's on the list: #875 Once Got 11 is released, I'll start to work on the enhancement issues and update our dependencies.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

[sindresorhus]

Got v11 is out: https://github.com/sindresorhus/got/releases/tag/v11.0.0 ✨