X Tutup
The Wayback Machine - https://web.archive.org/web/20220527234349/https://github.com/nodejs/node/pull/16495
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

Jump to bottom

cli: add --stack-trace-limit to NODE_OPTIONS #16495

Closed
wants to merge 1 commit into from
Closed

cli: add --stack-trace-limit to NODE_OPTIONS #16495

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Oct 25, 2017

I decided that it would make a lot of sense for me to set NODE_OPTIONS=--stack-trace-limit=100 on my development machine.

This code allows me to do that. :)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

cli

@addaleax addaleax added the cli label Oct 25, 2017
@nodejs-github-bot nodejs-github-bot added the c++ label Oct 25, 2017
@addaleax addaleax added semver-minor and removed c++ labels Oct 25, 2017
@Fishrock123
Copy link
Member

@Fishrock123 Fishrock123 commented Oct 25, 2017

Uh, yes please.

rmg
rmg approved these changes Oct 25, 2017
View changes
src/node.cc Outdated
@@ -4043,6 +4043,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
// V8 options (define with '_', which allows '-' or '_')
"--abort_on_uncaught_exception",
"--max_old_space_size",
"--stack_trace_limit"
Copy link
Contributor

@rmg rmg Oct 25, 2017

Choose a reason for hiding this comment

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

Nit: The line above this looks like this list was using trailing commas.

Copy link
Member Author

@addaleax addaleax Oct 25, 2017

Choose a reason for hiding this comment

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

thanks, done!

@addaleax
cli: add --stack-trace-limit to NODE_OPTIONS
c5725f8 
I decided that it would make a lot of sense for me to set
`NODE_OPTIONS=--stack-trace-limit=100` on my development machine.

This code allows me to do that. :)
@addaleax addaleax changed the title cli: add --stack-trace-limit to node options cli: add --stack-trace-limit to NODE_OPTIONS Oct 25, 2017
@mscdex mscdex added the v8 engine label Oct 26, 2017
@addaleax
Copy link
Member Author

@addaleax addaleax commented Oct 26, 2017

CI: https://ci.nodejs.org/job/node-test-commit/13486/

mhdawson
mhdawson approved these changes Oct 26, 2017
View changes
Copy link
Member

@mhdawson mhdawson left a comment

LGTM

@addaleax
Copy link
Member Author

@addaleax addaleax commented Nov 3, 2017

Landed in 89b3228

@addaleax addaleax closed this Nov 3, 2017
@addaleax addaleax deleted the node-options-stl branch Nov 3, 2017
addaleax added a commit that referenced this issue Nov 3, 2017
@addaleax
cli: add --stack-trace-limit to NODE_OPTIONS
89b3228 
I decided that it would make a lot of sense for me to set
`NODE_OPTIONS=--stack-trace-limit=100` on my development machine.

This code allows me to do that. :)

PR-URL: #16495
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@apapirovski
Copy link
Member

@apapirovski apapirovski commented Nov 3, 2017

@addaleax I think this might've landed with Windows tests being buggy, see:

not ok 71 parallel/test-cli-node-options
  ---
  duration_ms: 2.245
  severity: fail
  stack: |-
    assert.js:42
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: For "--stack-trace-limit=100", failed to find /(\s*at f \(\[eval\]:1:\d*\)\n){100}/ in: <
    [eval]:1
    (function f() { f(); })();

https://ci.nodejs.org/job/node-test-binary-windows/12473//console

addaleax added a commit to addaleax/node that referenced this issue Nov 3, 2017
@addaleax
test: fix test-cli-node-options on Windows
d762fc3 
nodejs#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: nodejs#16495
@addaleax addaleax mentioned this pull request Nov 3, 2017
Closed
3 tasks
addaleax added a commit that referenced this issue Nov 3, 2017
@addaleax
test: fix test-cli-node-options on Windows
3d4d5e0 
#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: #16495
PR-URL: #16709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
cjihrig pushed a commit to cjihrig/node that referenced this issue Nov 6, 2017
@addaleax @cjihrig
cli: add --stack-trace-limit to NODE_OPTIONS
7349d42 
I decided that it would make a lot of sense for me to set
`NODE_OPTIONS=--stack-trace-limit=100` on my development machine.

This code allows me to do that. :)

PR-URL: nodejs#16495
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this issue Nov 6, 2017
@addaleax @cjihrig
test: fix test-cli-node-options on Windows
26f1a1d 
nodejs#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: nodejs#16495
PR-URL: nodejs#16709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@cjihrig cjihrig mentioned this pull request Nov 6, 2017
Merged
cjihrig added a commit to cjihrig/node that referenced this issue Nov 7, 2017
@cjihrig
2017-11-07, Version 9.1.0 (Current)
9de633d 
Notable changes:

* CLI:
  - NODE_OPTIONS now supports the --stack-trace-limit option.
    nodejs#16495
* deps:
  - OpenSSL is upgraded to 1.0.2m
    nodejs#16691
* http:
  - A 'connect' event handler leak has been fixed.
    nodejs#16725
  - The 103 Early Hints status code is now supported.
    nodejs#16644

PR-URL: nodejs#16851
cjihrig added a commit that referenced this issue Nov 7, 2017
@cjihrig
2017-11-07, Version 9.1.0 (Current)
a77bfca 
Notable changes:

* CLI:
  - NODE_OPTIONS now supports the --stack-trace-limit option.
    #16495
* deps:
  - OpenSSL is upgraded to 1.0.2m
    #16691
* http:
  - A 'connect' event handler leak has been fixed.
    #16725
  - The 103 Early Hints status code is now supported.
    #16644

PR-URL: #16851
@gibfahn
Copy link
Member

@gibfahn gibfahn commented Nov 14, 2017

Should land with #16709 if it lands on LTS.

@gibfahn
Copy link
Member

@gibfahn gibfahn commented Jan 15, 2018

Release team were +1 on backporting to v6.x and v8.x.

MylesBorins pushed a commit that referenced this issue Jan 15, 2018
@addaleax @MylesBorins
cli: add --stack-trace-limit to NODE_OPTIONS
ec7996c 
I decided that it would make a lot of sense for me to set
`NODE_OPTIONS=--stack-trace-limit=100` on my development machine.

This code allows me to do that. :)

PR-URL: #16495
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@AndreasMadsen AndreasMadsen mentioned this pull request Jan 16, 2018
Closed
4 tasks
gibfahn pushed a commit to gibfahn/node that referenced this issue Jan 16, 2018
@addaleax @gibfahn
test: fix test-cli-node-options on Windows
439112a 
nodejs#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: nodejs#16495
PR-URL: nodejs#16709
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Jan 19, 2018

This has landed cleanly on v8.x but need a manual backport to v6.x. If we can't get this in before the minor release it will likely not land on 6.x at any point

gibfahn added a commit that referenced this issue Mar 6, 2018
@gibfahn
2018-03-06 Version 8.10.0 'Carbon' (LTS)
2fce636 
Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260)
  * re land npm 5.6.0 (Myles Borins) [#18625](#18625)
  * ICU 60 bump (Steven R. Loomis) [#16876](#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004)
  * use typed array stack as fast path (Anna Henningsen) [#17780](#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273)
  * separate missing from default context (Andreas Madsen) [#17273](#17273)
  * rename initTriggerId (Andreas Madsen) [#17273](#17273)
  * deprecate undocumented API (Andreas Madsen) [#16972](#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998)
  * add trace events to async_hooks (Andreas Madsen) [#15538](#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003)
  * add provider types for net server (Andreas Madsen) [#17157](#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033)
* module:
  * add builtinModules (Jon Moss) [#16386](#16386)
  * replace default paths in require.resolve() (cjihrig) [#17113](#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
  * add process.ppid (cjihrig) [#16839](#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [#17478](#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [#17158](#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196)

PR-URL: #18336
gibfahn added a commit that referenced this issue Mar 7, 2018
@gibfahn
2018-03-06 Version 8.10.0 'Carbon' (LTS)
fc9ddd9 
Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260)
  * re land npm 5.6.0 (Myles Borins) [#18625](#18625)
  * ICU 60 bump (Steven R. Loomis) [#16876](#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004)
  * use typed array stack as fast path (Anna Henningsen) [#17780](#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273)
  * separate missing from default context (Andreas Madsen) [#17273](#17273)
  * rename initTriggerId (Andreas Madsen) [#17273](#17273)
  * deprecate undocumented API (Andreas Madsen) [#16972](#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998)
  * add trace events to async_hooks (Andreas Madsen) [#15538](#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003)
  * add provider types for net server (Andreas Madsen) [#17157](#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033)
* module:
  * add builtinModules (Jon Moss) [#16386](#16386)
  * replace default paths in require.resolve() (cjihrig) [#17113](#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
  * add process.ppid (cjihrig) [#16839](#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [#17478](#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [#17158](#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196)

PR-URL: #18336
MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
@gibfahn @MayaLekova
2018-03-06 Version 8.10.0 'Carbon' (LTS)
185ac06 
Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [nodejs#16413](nodejs#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [nodejs#16413](nodejs#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [nodejs#18260](nodejs#18260)
  * re land npm 5.6.0 (Myles Borins) [nodejs#18625](nodejs#18625)
  * ICU 60 bump (Steven R. Loomis) [nodejs#16876](nodejs#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [nodejs#16130](nodejs#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [nodejs#17566](nodejs#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [nodejs#18004](nodejs#18004)
  * use typed array stack as fast path (Anna Henningsen) [nodejs#17780](nodejs#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * separate missing from default context (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * rename initTriggerId (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * deprecate undocumented API (Andreas Madsen) [nodejs#16972](nodejs#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [nodejs#16998](nodejs#16998)
  * add trace events to async_hooks (Andreas Madsen) [nodejs#15538](nodejs#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [nodejs#18003](nodejs#18003)
  * add provider types for net server (Andreas Madsen) [nodejs#17157](nodejs#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [nodejs#16495](nodejs#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [nodejs#17033](nodejs#17033)
* module:
  * add builtinModules (Jon Moss) [nodejs#16386](nodejs#16386)
  * replace default paths in require.resolve() (cjihrig) [nodejs#17113](nodejs#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109)
  * add process.ppid (cjihrig) [nodejs#16839](nodejs#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [nodejs#16267](nodejs#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [nodejs#17672](nodejs#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [nodejs#14772](nodejs#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [nodejs#17662](nodejs#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [nodejs#18463](nodejs#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [nodejs#17478](nodejs#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [nodejs#17158](nodejs#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [nodejs#12860](nodejs#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [nodejs#17196](nodejs#17196)

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

@rmg rmg

@jasnell jasnell

@Fishrock123 Fishrock123

@cjihrig cjihrig

@richardlau richardlau

@gireeshpunathil gireeshpunathil

@mhdawson mhdawson

@gibfahn gibfahn

Assignees
No one assigned
Labels
cli semver-minor v8 engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
X Tutup