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

addaleax · 2017-10-25T17:55:24Z

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

Fishrock123 · 2017-10-25T17:58:13Z

Uh, yes please.

rmg · 2017-10-25T20:24:37Z

src/node.cc

@@ -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"


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

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 · 2017-10-26T11:05:16Z

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

mhdawson

LGTM

addaleax · 2017-11-03T00:09:43Z

Landed in 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 · 2017-11-03T04:28:07Z

@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

nodejs#16495 broke the Windows build, accounting for `\r\n` newlines should fix it. Ref: nodejs#16495

#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>

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>

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>

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

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 · 2017-11-14T17:36:11Z

Should land with #16709 if it lands on LTS.

gibfahn · 2018-01-15T22:34:18Z

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

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>

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 · 2018-01-19T18:30:03Z

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

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

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

addaleax added the cli Issues and PRs related to the Node.js command line interface. label Oct 25, 2017

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 25, 2017

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

jasnell approved these changes Oct 25, 2017

View reviewed changes

richardlau approved these changes Oct 25, 2017

View reviewed changes

Fishrock123 approved these changes Oct 25, 2017

View reviewed changes

cjihrig approved these changes Oct 25, 2017

View reviewed changes

rmg approved these changes Oct 25, 2017

View reviewed changes

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 changed the title ~~cli: add --stack-trace-limit to node options~~ cli: add --stack-trace-limit to NODE_OPTIONS Oct 25, 2017

addaleax force-pushed the node-options-stl branch from 214e5ba to c5725f8 Compare October 25, 2017 21:04

mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Oct 26, 2017

gireeshpunathil approved these changes Oct 26, 2017

View reviewed changes

addaleax added lts-watch-v6.x labels Oct 26, 2017

mhdawson approved these changes Oct 26, 2017

View reviewed changes

gibfahn approved these changes Oct 28, 2017

View reviewed changes

addaleax closed this Nov 3, 2017

addaleax deleted the node-options-stl branch November 3, 2017 00:09

addaleax added a commit to addaleax/node that referenced this pull request Nov 3, 2017

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 mentioned this pull request Nov 3, 2017

test: fix test-cli-node-options on Windows #16709

Closed

3 tasks

cjihrig mentioned this pull request Nov 6, 2017

v9.1.0 proposal #16851

Merged

AndreasMadsen mentioned this pull request Jan 16, 2018

Backport async hooks to v8.x #18179

Closed

4 tasks

gibfahn added land-on-v8.x and removed lts-watch-v8.x labels Jan 16, 2018

MylesBorins added backport-requested-v6.x and removed lts-watch-v6.x labels Jan 19, 2018

MylesBorins added dont-land-on-v6.x and removed backport-requested-v6.x labels Feb 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

addaleax commented Oct 25, 2017

Fishrock123 commented Oct 25, 2017

rmg Oct 25, 2017

addaleax Oct 25, 2017

addaleax commented Oct 26, 2017

mhdawson left a comment

addaleax commented Nov 3, 2017

apapirovski commented Nov 3, 2017

gibfahn commented Nov 14, 2017

gibfahn commented Jan 15, 2018

MylesBorins commented Jan 19, 2018

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

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

Conversation

addaleax commented Oct 25, 2017

Checklist

Affected core subsystem(s)

Fishrock123 commented Oct 25, 2017

rmg Oct 25, 2017

Choose a reason for hiding this comment

addaleax Oct 25, 2017

Choose a reason for hiding this comment

addaleax commented Oct 26, 2017

mhdawson left a comment

Choose a reason for hiding this comment

addaleax commented Nov 3, 2017

apapirovski commented Nov 3, 2017

gibfahn commented Nov 14, 2017

gibfahn commented Jan 15, 2018

MylesBorins commented Jan 19, 2018