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

lib, test: migrate _http_outgoing.js's remaining errors to internal/errors.js #17837

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {

function write_(msg, chunk, encoding, callback, fromEnd) {
if (msg.finished) {
var err = new Error('write after end');
const err = new errors.Error('ERR_STREAM_WRITE_AFTER_END');
nextTick(msg.socket && msg.socket[async_id_symbol],
writeAfterEndNT.bind(msg),
err,
Expand Down Expand Up @@ -880,7 +880,7 @@ OutgoingMessage.prototype.flush = internalUtil.deprecate(function() {

OutgoingMessage.prototype.pipe = function pipe() {
// OutgoingMessage should be write-only. Piping from it is disabled.
this.emit('error', new Error('Cannot pipe, not readable'));
this.emit('error', new errors.Error('ERR_STREAM_CANNOT_PIPE'));
};

module.exports = {
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-http-res-write-after-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ const assert = require('assert');
const http = require('http');

const server = http.Server(common.mustCall(function(req, res) {
res.on('error', common.mustCall(function onResError(err) {
assert.strictEqual(err.message, 'write after end');
res.on('error', common.expectsError({
code: 'ERR_STREAM_WRITE_AFTER_END',
type: Error
}));

res.write('This should write.');
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-http-server-write-after-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@

const common = require('../common');
const http = require('http');
const assert = require('assert');

// Fix for https://github.com/nodejs/node/issues/14368

const server = http.createServer(handle);

function handle(req, res) {
res.on('error', common.mustCall((err) => {
assert.strictEqual(err.message, 'write after end');
Copy link
Contributor Author

@b0yfriend b0yfriend Dec 24, 2017

Choose a reason for hiding this comment

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

Oh, I missed this. I'll fix it soon!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

^^ Addressed this remark in the comment below.

common.expectsError({
code: 'ERR_STREAM_WRITE_AFTER_END',
type: Error
})(err);
server.close();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never mind. I thought I could remove the common.mustCall() on line 11, and simplify it to just common.expectsError().

That's what I did in test-pipe-outgoing-message-data-emitted-after-ended.js on lines 19-21.

However, I can't. This block performs server.close() in addition to common.expectsError(). That additional functionality prevents me from simplifying common.mustCall() => common.expectsError().

Copy link
Member

Choose a reason for hiding this comment

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

The way to handle that would be to register two on('error') handlers, one with the expectsError() check and the second with the server.close() call.

}));

Expand Down
13 changes: 6 additions & 7 deletions test/parallel/test-outgoing-message-pipe.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
'use strict';
const assert = require('assert');
const common = require('../common');
const OutgoingMessage = require('_http_outgoing').OutgoingMessage;

// Verify that an error is thrown upon a call to `OutgoingMessage.pipe`.

const outgoingMessage = new OutgoingMessage();
assert.throws(
common.mustCall(() => { outgoingMessage.pipe(outgoingMessage); }),
(err) => {
return ((err instanceof Error) && /Cannot pipe, not readable/.test(err));
},
'OutgoingMessage.pipe should throw an error'
common.expectsError(
() => { outgoingMessage.pipe(outgoingMessage); },
{
code: 'ERR_STREAM_CANNOT_PIPE',
type: Error
}
);
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';
const common = require('../common');
const http = require('http');
const assert = require('assert');
const stream = require('stream');

// Verify that when piping a stream to an `OutgoingMessage` (or a type that
Expand All @@ -17,8 +16,9 @@ const server = http.createServer(common.mustCall(function(req, res) {
process.nextTick(common.mustCall(() => {
res.end();
myStream.emit('data', 'some data');
res.on('error', common.mustCall(function(err) {
assert.strictEqual(err.message, 'write after end');
res.on('error', common.expectsError({
code: 'ERR_STREAM_WRITE_AFTER_END',
type: Error
}));

process.nextTick(common.mustCall(() => server.close()));
Expand Down