Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Tell the user why the HTTP response should not have a body. #2260

Closed
wants to merge 1 commit into from

4 participants

@partheseas

No description provided.

@isaacs

Is this error output really even necessary? It seems like node could just ignore the body silently.

@bnoordhuis

And get all kinds of confused bug reports? I'd rather keep it, it's a useful warning while developing.

@bnoordhuis bnoordhuis commented on the diff
lib/http.js
@@ -773,7 +773,10 @@ OutgoingMessage.prototype._flush = function() {
function ServerResponse(req) {
OutgoingMessage.call(this);
- if (req.method === 'HEAD') this._hasBody = false;
+ if (req.method === 'HEAD') {
+ this._hasBody = false;
+ this._noBodyReason = "HEAD";

I would strongly prefer it if you don't add another property to the ServerResponse class. It's never used in a well-debugged application so it only adds overhead.

It just seemed like the best way to do it. There are already so many I didn't think one more would be a blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@partheseas

If the error was removed, then a lot of people would probably submit issues of node not writing to the HTTP stream when told to.

@Nodejs-Jenkins
Collaborator

Can one of the admins verify this patch?

@isaacs

This no longer warns to stderr, and this pull is super old and crusty. Closing. Thanks for playing, please play again soon :)

@isaacs isaacs closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 3 deletions.
  1. +7 −3 lib/http.js
View
10 lib/http.js
@@ -592,7 +592,7 @@ OutgoingMessage.prototype.write = function(chunk, encoding) {
}
if (!this._hasBody) {
- console.error('This type of response MUST NOT have a body. ' +
+ console.error('This type of response (' + this._noBodyReason + ') MUST NOT have a body. ' +
'Ignoring write() calls.');
return true;
}
@@ -654,7 +654,7 @@ OutgoingMessage.prototype.end = function(data, encoding) {
}
if (data && !this._hasBody) {
- console.error('This type of response MUST NOT have a body. ' +
+ console.error('This type of response (' + this._noBodyReason + ') MUST NOT have a body. ' +
'Ignoring data passed to end().');
data = false;
}
@@ -773,7 +773,10 @@ OutgoingMessage.prototype._flush = function() {
function ServerResponse(req) {
OutgoingMessage.call(this);
- if (req.method === 'HEAD') this._hasBody = false;
+ if (req.method === 'HEAD') {
+ this._hasBody = false;
+ this._noBodyReason = "HEAD";

I would strongly prefer it if you don't add another property to the ServerResponse class. It's never used in a well-debugged application so it only adds overhead.

It just seemed like the best way to do it. There are already so many I didn't think one more would be a blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
if (req.httpVersionMajor < 1 || req.httpVersionMinor < 1) {
this.useChunkedEncodingByDefault = false;
@@ -858,6 +861,7 @@ ServerResponse.prototype.writeHead = function(statusCode) {
// consisting only of the Status-Line and optional headers, and is
// terminated by an empty line.
this._hasBody = false;
+ this._noBodyReason = statusCode;
}
// don't keep alive connections where the client expects 100 Continue
Something went wrong with that request. Please try again.