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

http: describe parse err in debug output #13206

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@sam-github
Member

sam-github commented May 24, 2017

parse errors are fairly invisible on the server, even with NODE_DEBUG, the actuall error is not reported, just that one ocurred. This patch helped me a lot.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

http

@sam-github sam-github requested a review from mscdex May 24, 2017

@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex May 24, 2017

Contributor

That's fine I suppose, you should probably add it in _http_client.js also for consistency.

Contributor

mscdex commented May 24, 2017

That's fine I suppose, you should probably add it in _http_client.js also for consistency.

@sam-github

This comment has been minimized.

Show comment
Hide comment
@sam-github
Member

sam-github commented May 24, 2017

@mscdex done

@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex May 24, 2017

Contributor

LGTM.

Contributor

mscdex commented May 24, 2017

LGTM.

@refack

refack approved these changes May 24, 2017

@sam-github

This comment has been minimized.

Show comment
Hide comment
@lpinca

lpinca approved these changes May 25, 2017

jasnell added a commit that referenced this pull request Jun 1, 2017

http: describe parse err in debug output
PR-URL: #13206
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Jun 1, 2017

Member

Landed in 3ffba3d6 3fba3d6

Member

jasnell commented Jun 1, 2017

Landed in 3ffba3d6 3fba3d6

@jasnell jasnell closed this Jun 1, 2017

@jasnell jasnell reopened this Jun 1, 2017

@jasnell jasnell closed this Jun 1, 2017

@jasnell jasnell reopened this Jun 1, 2017

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Jun 1, 2017

Member

eh... no idea what just happened right there.

Member

jasnell commented Jun 1, 2017

eh... no idea what just happened right there.

@jasnell jasnell closed this Jun 1, 2017

@refack

This comment has been minimized.

Show comment
Hide comment
@refack

refack Jun 1, 2017

Member

edited commit hash

Member

refack commented Jun 1, 2017

edited commit hash

jasnell added a commit that referenced this pull request Jun 5, 2017

http: describe parse err in debug output
PR-URL: #13206
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>

@jasnell jasnell referenced this pull request Jun 5, 2017

Closed

8.1.0 Proposal #13483

@sam-github sam-github deleted the sam-github:describe-parse-error branch Jun 14, 2017

@gibfahn gibfahn referenced this pull request Jun 15, 2017

Closed

Auditing for 6.11.1 #230

2 of 3 tasks complete
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Jul 17, 2017

Member

Should this be backported to v6.x?

Member

MylesBorins commented Jul 17, 2017

Should this be backported to v6.x?

sam-github added a commit to sam-github/node that referenced this pull request Jul 21, 2017

http: describe parse err in debug output
PR-URL: nodejs#13206
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@sam-github

This comment has been minimized.

Show comment
Hide comment
@sam-github

sam-github Jul 21, 2017

Member

@MylesBorins yes, and I did.

Member

sam-github commented Jul 21, 2017

@MylesBorins yes, and I did.

MylesBorins added a commit that referenced this pull request Jul 21, 2017

http: describe parse err in debug output
Backport-PR-URL: #14416
PR-URL: #13206
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>

@MylesBorins MylesBorins referenced this pull request Jul 21, 2017

Merged

v6.11.2 proposal #14356

MylesBorins added a commit that referenced this pull request Jul 31, 2017

http: describe parse err in debug output
Backport-PR-URL: #14416
PR-URL: #13206
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>

MylesBorins added a commit that referenced this pull request Aug 1, 2017

2017-08-01, Version 6.11.2 'Boron' (LTS)
This LTS release comes with 221 commits. This includes 80 which are
test related, 52 which are doc related, 32 which are build / tool
related and 10 commits which are updates to dependencies.

Notable Changes:

* configure:
  - add mips64el to valid_arch (Aditya Anand)
    - #13620
* crypto:
  - Updated root certificates based on [NSS 3.30] (Ben Noordhuis)
    - #13279
    - #12402
    - https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.30_release_notes
* deps:
  - upgrade OpenSSL to version 1.0.2.l (Shigeki Ohtsu)
    - #12913
* http:
  - parse errors are now reported when NODE_DEBUG=http (Sam Roberts)
    - #13206
  - Agent construction can now be envoked without `new` (cjihrig)
    - #12927
* zlib:
  - node will now throw an Error when zlib rejects the value of windowBits,
    instead of crashing (Alexey Orlenko)
    - #13098

PR-URL: #14356

MylesBorins added a commit that referenced this pull request Aug 1, 2017

2017-08-01, Version 6.11.2 'Boron' (LTS)
This LTS release comes with 221 commits. This includes 80 which are
test related, 52 which are doc related, 32 which are build / tool
related and 10 commits which are updates to dependencies.

Notable Changes:

* configure:
  - add mips64el to valid_arch (Aditya Anand)
    - #13620
* crypto:
  - Updated root certificates based on [NSS 3.30] (Ben Noordhuis)
    - #13279
    - #12402
    - https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.30_release_notes
* deps:
  - upgrade OpenSSL to version 1.0.2.l (Shigeki Ohtsu)
    - #12913
* http:
  - parse errors are now reported when NODE_DEBUG=http (Sam Roberts)
    - #13206
  - Agent construction can now be envoked without `new` (cjihrig)
    - #12927
* zlib:
  - node will now throw an Error when zlib rejects the value of windowBits,
    instead of crashing (Alexey Orlenko)
    - #13098

PR-URL: #14356
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment