Skip to content

Conversation

@romanshoryn
Copy link
Contributor

@romanshorynromanshoryn commented Jul 9, 2017

Hi everyone!

I fixed the wrong sentence in the Error.captureStackTrace(targetObject[, constructorOpt]) part of api/errors doc.

Fixes: #5675

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)

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jul 9, 2017
@romanshorynromanshoryn changed the title doc: fix doc:Error:captureStackTrace descriptiondoc: fix Error:captureStackTrace descriptionJul 9, 2017

The first line of the trace, instead of being prefixed with `ErrorType:
message`, will be the result of calling `targetObject.toString()`.
The first line of the trace will be prefixed with `ErrorType.name: message`.
Copy link
Member

Choose a reason for hiding this comment

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

It will be myObject.name if myObject.message doesn't exist, or ${myObject.name}: ${myObject.message} if it does. I think it's the ErrorType that's tripping me up a bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.
I'd assume this paragraph and the snippet above it have "evolved" separately and have become disjoint.
the actual formula is:

`${myObject.name||'Error'}${String(myObject.message) ? (': '+String(myObject.message)) : ''}`

But IMHO `${myObject.name}: ${myObject.message}` is good enough

@romanshoryn
Copy link
ContributorAuthor

@refack , @TimothyGu thanks for the review. I've fixed the line.

@refackrefack self-assigned this Jul 12, 2017
refack pushed a commit to refack/node that referenced this pull request Jul 12, 2017
PR-URL: nodejs#14150Fixes: nodejs#5675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
@refack
Copy link
Contributor

Landed in 7f26a29

@refackrefack closed this Jul 12, 2017
@addaleaxaddaleax mentioned this pull request Jul 18, 2017
addaleax pushed a commit that referenced this pull request Jul 18, 2017
PR-URL: #14150Fixes: #5675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
PR-URL: #14150Fixes: #5675 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
@MylesBorins
Copy link
Contributor

is this relevant to behavior in 6.x?

@MylesBorins
Copy link
Contributor

ping

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.errorsIssues and PRs related to JavaScript errors originated in Node.js core.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc:Error:captureStackTrace description inaccurate

7 participants

@romanshoryn@refack@MylesBorins@jasnell@TimothyGu@kunalspathak@nodejs-github-bot