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

issue #953: stop using writeHead #1182

Closed

Conversation

jakefurler
Copy link

Using object.keys in web-incoming.js results in a non-deterministic ordering of keys, which means that in web-outgoing writeHead might be called before setHeader, which throws an error.

Example of a server that fails to send a response :

const http = require('http');

const server = http.createServer((req, res) => {
  res.writeHead(200);
  res.setHeader('Content-Type', 'text/html');
  res.setHeader('X-Foo', 'bar');
  res.end('ok');
});

server.listen(3000);

This will throw Error: Can't set headers after they are sent. when a request is made to it. If we change it to the following, it will work:

const http = require('http');

const server = http.createServer((req, res) => {
  res.statusCode = 200;
  // res.writeHead(200);
  res.setHeader('Content-Type', 'text/html');
  res.setHeader('X-Foo', 'bar');
  res.end('ok');
});

server.listen(3000);

object.keys in web-incoming.js results in a non-deterministic ordering of keys, which means that in web-outgoing writeHead might be called before setHeader, which throws an error
@jakefurler
Copy link
Author

@jcrugzz can you please review this? It's causing issues in one of my production services, and I'm having to work around it at this point in time. I think merging it would solve a lot of issues people seem to be running into.

@jcrugzz
Copy link
Contributor

jcrugzz commented Apr 19, 2018

Thanks for the contribution @jakefurler! I cherry-picked your commit to my maintenance branch which i plan to merge later tonight.

@jcrugzz jcrugzz closed this Apr 19, 2018
@jcrugzz
Copy link
Contributor

jcrugzz commented Apr 20, 2018

superseded by #1251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants