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

doc: add simple http clientError example #5248

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Feb 15, 2016

The clientError event allows proper http 4xx responses to
be returned when a parse error occurs, but the documentation
did not demonstrate how to use it.

/cc @nodejs/http @indutny

@jasnell jasnell added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Feb 15, 2016
});
server.on('clientError', (err, socket) => {
socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
server.close();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that server.close() is relevant here.

@indutny
Copy link
Member

indutny commented Feb 15, 2016

Otherwise LGTM

The clientError event allows proper http 4xx responses to
be returned when a parse error occurs, but the documentation
did not demonstrate how to use it.
@jasnell jasnell force-pushed the http-client-error-doc branch from 329aa60 to 649c3b0 Compare February 16, 2016 16:44
@jasnell
Copy link
Member Author

jasnell commented Feb 16, 2016

@indutny ... removed the server.close() (It was an artifact from the test case since I literally just copied it over ;-) ...)

@ChALkeR
Copy link
Member

ChALkeR commented Feb 16, 2016

LGTM

@indutny
Copy link
Member

indutny commented Feb 16, 2016

LGTM, please land ;)

jasnell added a commit that referenced this pull request Feb 17, 2016
The clientError event allows proper http 4xx responses to
be returned when a parse error occurs, but the documentation
did not demonstrate how to use it.

PR-URL: #5248
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Feb 17, 2016

Landed in 9b6440a
@indutny ... I wanted to confirm that this mechanism is not available in v4.x, correct?

@jasnell
Copy link
Member Author

jasnell commented Feb 17, 2016

btw, I believe this currently only works in master...

@indutny
Copy link
Member

indutny commented Feb 17, 2016

Only master, right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants