Skip to content

stream: eos on closed #28748

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.

Sign up for GitHub

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

Closed
ronag wants to merge 12 commits into from
Closed

stream: eos on closed #28748

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
820074e
stream: eos on closed
ronag Jul 18, 2019
729cd05
fixup: check ended and finished
ronag Aug 27, 2019
e662280
fixup: http response doesn't have destroyed, use aborted instead
ronag Aug 27, 2019
3ec6ecc
fixup: tests
ronag Aug 28, 2019
27a9624
fixup: more tests
ronag Aug 28, 2019
5641590
fixup: merge #28720
ronag Sep 1, 2019
41a692a
fixup: sligthly simplify and add more tests
ronag Sep 1, 2019
3be0454
fixup: test descriptions
ronag Sep 1, 2019
53c04a3
fixup: linting
ronag Sep 1, 2019
e8284ec
fixup: merge duplicate test files
ronag Sep 1, 2019
a4542b7
fixup: comments styling
ronag Sep 1, 2019
9b5a84e
fixup: typo when merging tests
ronag Sep 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixup: linting
  • Loading branch information
@ronag
ronag committed Sep 2, 2019
commit 53c04a393cf58100dc1b12b362589b774c28f027
16 changes: 8 additions & 8 deletions test/parallel/test-http-client-finished.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const { finished } = require('stream');
}

{
// abort before end
// Test abort before end.

const server = http.createServer(function(req, res) {
res.write('test');
Expand All @@ -76,10 +76,10 @@ const { finished } = require('stream');
server.listen(0, common.mustCall(function() {
const req = http.request({
port: this.address().port
}).on('response', common.mustCall(res => {
}).on('response', common.mustCall((res) => {
req.abort();
finished(res, common.mustCall(() => {
finished(res, common.mustCall(() =>{
finished(res, common.mustCall(() => {
server.close();
}));
}));
Expand All @@ -88,7 +88,7 @@ const { finished } = require('stream');
}

{
// destroy before end
// Test destroy before end.

const server = http.createServer(function(req, res) {
res.write('test');
Expand All @@ -97,7 +97,7 @@ const { finished } = require('stream');
server.listen(0, common.mustCall(function() {
http.request({
port: this.address().port
}).on('response', common.mustCall(res => {
}).on('response', common.mustCall((res) => {
// TODO(ronag): Bug? Won't emit 'close' unless read.
res.on('data', () => {});
Copy link
Copy Markdown
Member Author

@ronag ronag Aug 28, 2019

Choose a reason for hiding this comment

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

@addaleax: Isn't it a bit strange that the 'data' handler is required here in order for the response to end/close? Likewise in a test further down.

Copy link
Copy Markdown
Member Author

@ronag ronag Sep 1, 2019

Choose a reason for hiding this comment

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

ping @addaleax? I think this is out of the scope of this PR. Please just confirm whether this is strange and we can create a new issue for later investigation.

res.destroy();
Expand All @@ -111,7 +111,7 @@ const { finished } = require('stream');
}

{
// finish after end
// Test finish after end.

const server = http.createServer(function(req, res) {
res.end('asd');
Expand All @@ -120,13 +120,13 @@ const { finished } = require('stream');
server.listen(0, common.mustCall(function() {
http.request({
port: this.address().port
}).on('response', common.mustCall(res => {
}).on('response', common.mustCall((res) => {
// TODO(ronag): Bug? Won't emit 'close' unless read.
res.on('data', () => {});
finished(res, common.mustCall(() => {
finished(res, common.mustCall(() => {
server.close();
}))
}));
}));
})).end();
}));
Expand Down