http: throw error when attempting to write to response with no body

[mertcanaltin]

Previously, when the _hasBody property of the response object was false, the code simply ignored any data being written to the response. This behavior was changed to throw an error instead, which is more appropriate for responses that are not allowed to have a body

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[ronag]

This needs a test. After which, I think you will notice that this does not work since all implementations override this method.

[mertcanaltin]

This needs a test. After which, I think you will notice that this does not work since all implementations override this method.

Do I need to create a new test, I would appreciate it if you could inform me about it.

[ronag]

Yes, please create a new test.

[mertcanaltin]

Yes, please create a new test.

I added a test

[ronag]

Yes, please create a new test.

I added a test

The test is failing.

[ronag]

You should test the public api not the internal one.

[HinataKah0]

I think you can try looking into how the response body is sent, the API that devs use to write to the response body. It should be this API. From there, you can try to trace the code and understand how it works.

[HinataKah0]

Ah, actually I meant about where to start to understand about the necessary changes.

What I suggested is: because we want to prevent writing to the response body when the request method is HEAD, we can start by looking at the write API itself.

You can search for _implicitHeader in the codebase and find that OutgoingMessage's _implicitHeader is overridden by ServerResponse. So I think writing changes in _implicitHeader is not going to work.

About writing test, I think you can refer to existing tests, you can search in the codebase with the keyword "test-http" and there are plenty of them.

And you can try running the tests in local first, see this.

I hope that helps 😄

[HinataKah0]

I think we want to verify that HEAD can't write. So I am not sure why we are only testing GET here.

[mertcanaltin]

@ronag
does defining the error as a class make it flexible for further steps?

class HttpBodyNotAllowedError extends Error {
  constructor(message) {
    super(message);
    this.name = 'HttpBodyNotAllowedError';
    this.code = 'ERR_HTTP_BODY_NOT_ALLOWED';
  }
}