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

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

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

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 Issues and PRs related to the Node.js command line interface. label Oct 25, 2017
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 25, 2017
@addaleax addaleax added semver-minor PRs that contain new features and should be released in the next minor version. and removed c++ Issues and PRs that require attention from people who are familiar with C++. labels Oct 25, 2017
@Fishrock123
Copy link
Contributor

Uh, yes please.

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

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

Choose a reason for hiding this comment

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

thanks, done!

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 Issues and PRs related to the V8 dependency. label Oct 26, 2017
@addaleax
Copy link
Member Author

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@addaleax
Copy link
Member Author

addaleax commented Nov 3, 2017

Landed in 89b3228

@addaleax addaleax closed this Nov 3, 2017
@addaleax addaleax deleted the node-options-stl branch November 3, 2017 00:09
addaleax added a commit that referenced this pull request Nov 3, 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. :)

PR-URL: #16495
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ryan Graham <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@apapirovski
Copy link
Member

@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 pull request Nov 3, 2017
nodejs#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: nodejs#16495
addaleax added a commit that referenced this pull request Nov 3, 2017
#16495 broke the Windows build,
accounting for `\r\n` newlines should fix it.

Ref: #16495
PR-URL: #16709
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 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. :)

PR-URL: nodejs#16495
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ryan Graham <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@cjihrig cjihrig mentioned this pull request Nov 6, 2017
cjihrig added a commit to cjihrig/node that referenced this pull request Nov 7, 2017
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 pull request Nov 7, 2017
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 commented Nov 14, 2017

Should land with #16709 if it lands on LTS.

@gibfahn
Copy link
Member

gibfahn commented Jan 15, 2018

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

MylesBorins pushed a commit that referenced this pull request Jan 15, 2018
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ryan Graham <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@AndreasMadsen AndreasMadsen mentioned this pull request Jan 16, 2018
4 tasks
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 16, 2018
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@MylesBorins
Copy link
Contributor

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 pull request Mar 6, 2018
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 pull request Mar 7, 2018
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 pull request May 8, 2018
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
Labels
cli Issues and PRs related to the Node.js command line interface. semver-minor PRs that contain new features and should be released in the next minor version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.