error-reporting: Fix report() silently failing
#2363
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
| "repository": "GoogleCloudPlatform/google-cloud-node", | ||
| "scripts": { | ||
| "test": "nyc --exclude=\"fuzzer.js\" mocha ./test/unit/**/*.js", | ||
| "test": "nyc --exclude=\"fuzzer.js\" mocha ./test/unit/*.js ./test/unit/**/*.js", |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
ofrobots
reviewed
Jun 9, 2017
Contributor
ofrobots
left a comment
ofrobots
left a comment
There was a problem hiding this comment.
Added some comments. Some of them were present in the original code, so it might be acceptable to do them in a follow-on (ie. keep this as a refactor, and make logical changes in a follow-on).
| * Constructs a string representation of the stack trace at the point where | ||
| * this function was invoked. Note that the stack trace will not include any | ||
| * references to this function itself. | ||
| * @function buildStackTrace |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| var fauxError = new Error(''); | ||
| var fullStack = fauxError.stack.split('\n'); | ||
| var cleanedStack = fullStack.slice(2).join('\n'); | ||
| var cleanedStack = buildStackTrace('', 1); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| function populateErrorMessage(ob, em) { | ||
| if (ob === null || ob === undefined) { | ||
| populateFromUnknown(ob, em); | ||
| } else if (ob instanceof Error) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| * @returns {Undefined} - does not return anything | ||
| */ | ||
| function populateFromNumber(num, errorMessage) { | ||
| var message = isNumber(num) && isFunction(num.toString) ? num.toString() : ''; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| * @returns {Undefined} - does not return anything | ||
| */ | ||
| function populateFromString(str, errorMessage) { | ||
| errorMessage.setMessage(buildStackTrace(str, 3)); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
If the `report()` method is called with a number, an associated error never appears in the error reporting console, and no log message is printed to the user to indicate why the error has not appeared.
The `Error.captureStackTrace` method is now used in the `buildStackTrace` function to create a stack trace.
The code for verifiying that stack traces don't contain error-reporting specific frames has been consolidated into a single location.
DominicKramer
force-pushed
the
bug/reporting-non-errors-silently-fails
branch
from
28226ff to
3df370b
Compare
ofrobots
approved these changes
Jun 15, 2017
In particular, the `buildStackTrace` was updated to ensure that no error-reporting specific frames are included in the built stack trace. In addition, the system-tests have been updated to ensure error-reporting related frames, and only those frames, are removed from the built stack trace.
ofrobots
approved these changes
Jun 15, 2017
Contributor
Author
|
@stephenplusplus This is ready to land. |
This was referenced Jul 5, 2017
GautamSharda
pushed a commit
that referenced
this pull request
Jan 14, 2026
* error-reporting: Fix `report()` silently failing If the `report()` method is called with a number, an associated error never appears in the error reporting console, and no log message is printed to the user to indicate why the error has not appeared. * Fix linting errors * Add the new `build-stack-trace.js` file * Add more system tests for `errors.report()` * Rename errorHandlerRouter to populateErrorMessage * Rename error-router to populate-error-message * Reorganize the populate-error-message tests * Update buildStackTrace to use captureStackTrace The `Error.captureStackTrace` method is now used in the `buildStackTrace` function to create a stack trace. * Simplify the system tests The code for verifiying that stack traces don't contain error-reporting specific frames has been consolidated into a single location. * Consolidate code in populate-error-message.js * Make the `buildStackTrace` more robust In particular, the `buildStackTrace` was updated to ensure that no error-reporting specific frames are included in the built stack trace. In addition, the system-tests have been updated to ensure error-reporting related frames, and only those frames, are removed from the built stack trace.
GautamSharda
pushed a commit
that referenced
this pull request
Jan 15, 2026
* error-reporting: Fix `report()` silently failing If the `report()` method is called with a number, an associated error never appears in the error reporting console, and no log message is printed to the user to indicate why the error has not appeared. * Fix linting errors * Add the new `build-stack-trace.js` file * Add more system tests for `errors.report()` * Rename errorHandlerRouter to populateErrorMessage * Rename error-router to populate-error-message * Reorganize the populate-error-message tests * Update buildStackTrace to use captureStackTrace The `Error.captureStackTrace` method is now used in the `buildStackTrace` function to create a stack trace. * Simplify the system tests The code for verifiying that stack traces don't contain error-reporting specific frames has been consolidated into a single location. * Consolidate code in populate-error-message.js * Make the `buildStackTrace` more robust In particular, the `buildStackTrace` was updated to ensure that no error-reporting specific frames are included in the built stack trace. In addition, the system-tests have been updated to ensure error-reporting related frames, and only those frames, are removed from the built stack trace.
4 tasks
GautamSharda
pushed a commit
that referenced
this pull request
Feb 5, 2026
* error-reporting: Fix `report()` silently failing If the `report()` method is called with a number, an associated error never appears in the error reporting console, and no log message is printed to the user to indicate why the error has not appeared. * Fix linting errors * Add the new `build-stack-trace.js` file * Add more system tests for `errors.report()` * Rename errorHandlerRouter to populateErrorMessage * Rename error-router to populate-error-message * Reorganize the populate-error-message tests * Update buildStackTrace to use captureStackTrace The `Error.captureStackTrace` method is now used in the `buildStackTrace` function to create a stack trace. * Simplify the system tests The code for verifiying that stack traces don't contain error-reporting specific frames has been consolidated into a single location. * Consolidate code in populate-error-message.js * Make the `buildStackTrace` more robust In particular, the `buildStackTrace` was updated to ensure that no error-reporting specific frames are included in the built stack trace. In addition, the system-tests have been updated to ensure error-reporting related frames, and only those frames, are removed from the built stack trace.
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the
report()method is called with a number, an associatederror never appears in the error reporting console, and no log
message is printed to the user to indicate why the error has not
appeared.