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

.end(data, callback) doesn't work; .end(callback) doesn't work when recording #1549

Closed
paulmelnikow opened this issue May 9, 2019 · 2 comments

Comments

@paulmelnikow
Copy link
Member

paulmelnikow commented May 9, 2019

What is the expected behavior?

Calling .end(encoding, callback) .end(data, callback) on overridden requests, or when recording, should have the same behavior as a real client request.

When recording, calling .end(callback) should have the same behavior as a real client request.

See #1542 which fixed the .end(callback) case for overridden requests.

First reported in #1509.

What is the actual behavior?

Not sure.

Possible solution

Adapt Node's code for shuffling argument: #1509 (comment)

Does the bug have a test case?

Not yet.

@paulmelnikow paulmelnikow changed the title .end(encoding, callback) doesn't work; .end(callback) doesn't work when recording .end(data, callback) doesn't work; .end(callback) doesn't work when recording May 9, 2019
mastermatt added a commit to mastermatt/nock that referenced this issue Jun 21, 2019
Fixes nock#1549

The method now correctly accepts all the permutations allowed.
request.end(data, encoding, callback)
request.end(data, callback)
request.end(data, encoding)
request.end(data)
request.end(callback)
request.end()

And a few tests were added to ensure all cases are explicitly covered.
paulmelnikow pushed a commit that referenced this issue Jul 7, 2019
* Fix typo about wrapping request.end.

I'm making this change it's own commit so I have a place to comment the
findings.

Digging through Node git history, I found the change that created the
breaking change in nock (ref nock PR 929).
nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779

Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`,
would literally do what it says in the docs if data was provided. It
would call `this.write(data, encoding)`. This meant that nock could wrap
only the `write` method when recording and gather all the chunks even if
the last chunk was sent to `end`. But, the above changed that to call an
internal function dual used by `end` and `write`.

* fix: request.end accepted arguments.

Fixes #1549

The method now correctly accepts all the permutations allowed.
request.end(data, encoding, callback)
request.end(data, callback)
request.end(data, encoding)
request.end(data)
request.end(callback)
request.end()

And a few tests were added to ensure all cases are explicitly covered.
@nockbot
Copy link
Collaborator

nockbot commented Jul 7, 2019

🎉 This issue has been resolved in version 11.0.0-beta.21 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nockbot
Copy link
Collaborator

nockbot commented Aug 13, 2019

🎉 This issue has been resolved in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

gr2m pushed a commit that referenced this issue Sep 4, 2019
* Fix typo about wrapping request.end.

I'm making this change it's own commit so I have a place to comment the
findings.

Digging through Node git history, I found the change that created the
breaking change in nock (ref nock PR 929).
nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779

Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`,
would literally do what it says in the docs if data was provided. It
would call `this.write(data, encoding)`. This meant that nock could wrap
only the `write` method when recording and gather all the chunks even if
the last chunk was sent to `end`. But, the above changed that to call an
internal function dual used by `end` and `write`.

* fix: request.end accepted arguments.

Fixes #1549

The method now correctly accepts all the permutations allowed.
request.end(data, encoding, callback)
request.end(data, callback)
request.end(data, encoding)
request.end(data)
request.end(callback)
request.end()

And a few tests were added to ensure all cases are explicitly covered.
gr2m pushed a commit that referenced this issue Sep 4, 2019
* Fix typo about wrapping request.end.

I'm making this change it's own commit so I have a place to comment the
findings.

Digging through Node git history, I found the change that created the
breaking change in nock (ref nock PR 929).
nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779

Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`,
would literally do what it says in the docs if data was provided. It
would call `this.write(data, encoding)`. This meant that nock could wrap
only the `write` method when recording and gather all the chunks even if
the last chunk was sent to `end`. But, the above changed that to call an
internal function dual used by `end` and `write`.

* fix: request.end accepted arguments.

Fixes #1549

The method now correctly accepts all the permutations allowed.
request.end(data, encoding, callback)
request.end(data, callback)
request.end(data, encoding)
request.end(data)
request.end(callback)
request.end()

And a few tests were added to ensure all cases are explicitly covered.
gr2m pushed a commit that referenced this issue Sep 5, 2019
* Fix typo about wrapping request.end.

I'm making this change it's own commit so I have a place to comment the
findings.

Digging through Node git history, I found the change that created the
breaking change in nock (ref nock PR 929).
nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779

Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`,
would literally do what it says in the docs if data was provided. It
would call `this.write(data, encoding)`. This meant that nock could wrap
only the `write` method when recording and gather all the chunks even if
the last chunk was sent to `end`. But, the above changed that to call an
internal function dual used by `end` and `write`.

* fix: request.end accepted arguments.

Fixes #1549

The method now correctly accepts all the permutations allowed.
request.end(data, encoding, callback)
request.end(data, callback)
request.end(data, encoding)
request.end(data)
request.end(callback)
request.end()

And a few tests were added to ensure all cases are explicitly covered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants