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: Document the --stack-trace-limit cli flag. #6377

Closed
wants to merge 1 commit into from
Closed

doc: Document the --stack-trace-limit cli flag. #6377

wants to merge 1 commit into from

Conversation

lance
Copy link
Member

@lance lance commented Apr 25, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Partially fixes #6271. This commit
simply documents the V8 cli option --stack_trace_limit but specifies
dashes instead of underscores, which is the Node.js cli convention. I
did not write a test for this, since the behavior hasn't changed. But I
did validate locally that --stack-trace-limit=N works as now
documented.

The issue noted also has a feature request for functional changes to
the stack trace display, potentially filtering Node.js internals. And also,
potentially increasing the Error.stackTraceLimit just for the first tick
for Module.runMain. This PR does not address those requests since
it wasn't really clear to me from the conversation if those were really
desired or needed.

Partially fixes #6271. This commit
simply documents the V8 cli option `--stack_trace_limit` but specifies
dashes instead of underscores, which is the Node.js cli convention. I
did not write a test for this, since the behavior hasn't changed. But I
did validate locally that `--stack-trace-limit=N` works as now
documented.
@mscdex mscdex added the doc Issues and PRs related to the documentations. label Apr 25, 2016
@jasnell
Copy link
Member

jasnell commented Apr 25, 2016

hmm.. while I don't have a problem with this particular change, we don't typically document v8 command line flags alongside the set provided by Node.js itself. The reason is that those are not under our control to change but as soon as we document it they become our responsibility to maintain. If the v8 team decided to change it's flag, for instance, we'd be forced to either find a way to continue supporting it or do a quick deprecation (which is never ideal).

@nodejs/ctc thoughts?

@mscdex
Copy link
Contributor

mscdex commented Apr 25, 2016

I'm -1 on documenting specific v8 options.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 25, 2016

Also -1 on documenting V8.

@indutny
Copy link
Member

indutny commented Apr 25, 2016

-1

@lance
Copy link
Member Author

lance commented Apr 25, 2016

OK - I was just responding to the issue, trying to hit some low hanging fruit. I am happy to close the PR. Any thoughts on the feature request(s) - either the internals filtering or the increased limit? Personally, I think the increased limit is not the right path to take. Not sure about internals filtering.

@lance
Copy link
Member Author

lance commented Apr 25, 2016

We're at -3.5 so far. Closing...

@lance lance closed this Apr 25, 2016
@indutny
Copy link
Member

indutny commented Apr 25, 2016

@lance filtering internals is not an option in my opinion. This will tremendously complicate the node.js bug report process, since we won't be able to tell what failed and where it broke.

@lance
Copy link
Member Author

lance commented Apr 25, 2016

@indutny that works for me and fwiw, I agree with earlier sentiment that this info is useful for more than just core committers. Often times it's helped me figure out wtf I was doing wrong in application code. As I said, I was just swinging for low hanging fruit.

@indutny
Copy link
Member

indutny commented Apr 25, 2016

@lance no problem at all, sorry it gone this way!

@trevnorris
Copy link
Contributor

Should we possibly make a note somewhere that v8 options accept both underscores and dashes?

@cjihrig cjihrig mentioned this pull request Apr 27, 2016
@stevemao
Copy link
Contributor

Should we possibly make a note somewhere that v8 options accept both underscores and dashes?

I think we should do that. CC @nodejs/documentation

Fishrock123 added a commit to Fishrock123/node that referenced this pull request May 9, 2016
This adds docs to the man page and online cli docs that v8 options can
be used with either dashes or underscores.

Refs: nodejs#6377 (comment)
PR-URL: nodejs#6532
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request May 17, 2016
This adds docs to the man page and online cli docs that v8 options can
be used with either dashes or underscores.

Refs: #6377 (comment)
PR-URL: #6532
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve stack traces for exceptions thrown at a top-level
7 participants