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

response.writeHead() does not default statusMessage if it's undefined #32395

Closed
spazmodius opened this issue Mar 20, 2020 · 4 comments · Fixed by #46173
Closed

response.writeHead() does not default statusMessage if it's undefined #32395

spazmodius opened this issue Mar 20, 2020 · 4 comments · Fixed by #46173
Labels
http Issues or PRs related to the http subsystem.

Comments

@spazmodius
Copy link

  • Version: 12.16.1
  • Platform: Windows
  • Subsystem: http

response.writeHead(statusCode[, statusMessage][, headers])

works as expected if statusMessage is omitted, supplying a default value.
However, if undefined is passed as a placeholder then it unexpectedly ignores the headers argument.

Why would I want this? Imagine a function like:

function send({ statusCode, statusMessage, headers, body } = { statusCode: 404 }, res) {
  return new Promise((resolve, reject) => {
    res.writeHead(statusCode, statusMessage, headers)
      .end(body)
      .on('finish', resolve)
      .on('error', reject)
  })
}

You can see I depend on a single signature to work whether statusMessage is supplied or not. Otherwise, I have to:

  if (statusMessage)
    res.writeHead(statusCode, statusMessage, headers)
  else
    res.writeHead(statusCode, headers)

Not the end of the world, but it lacks elegance.

Maybe this is not a bug, but wrong expectations?
Perhaps. But this expectation aligns with how javascript default parameters behave.

@himself65
Copy link
Member

I'm working on this.

@himself65
Copy link
Member

Maybe this is not a bug, but wrong expectations?

Yes, this not a bug, just we have a different parameter processing

@spazmodius
Copy link
Author

So, actually, this title is inaccurate: response.writeHead() does not default statusMessage if it's undefined.

statusMessage is defaulted correctly by passing the value undefined. The problem is that then the headers, if passed, is ignored. So a more accurate title would be: response.writeHead() misbehaves when statusMessage is undefined

@himself65
Copy link
Member

So, actually, this title is inaccurate: response.writeHead() does not default statusMessage if it's undefined.

statusMessage is defaulted correctly by passing the value undefined. The problem is that then the headers, if passed, is ignored. So a more accurate title would be: response.writeHead() misbehaves when statusMessage is undefined

yes, and i have fixed this in that PR

@targos targos added the http Issues or PRs related to the http subsystem. label Dec 26, 2020
nodejs-github-bot pushed a commit that referenced this issue Jan 17, 2023
PR-URL: #46173
Fixes: #32395
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
RafaelGSS pushed a commit to RafaelGSS/node that referenced this issue Jan 17, 2023
PR-URL: nodejs#46173
Fixes: nodejs#32395
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 20, 2023
PR-URL: #46173
Fixes: #32395
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
juanarbol pushed a commit that referenced this issue Jan 26, 2023
PR-URL: #46173
Fixes: #32395
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
juanarbol pushed a commit that referenced this issue Jan 31, 2023
PR-URL: #46173
Fixes: #32395
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
3 participants