Capture exception message in browser telemetry#304
Open
archandatta wants to merge 2 commits into
Open
Conversation
Runtime.exceptionThrown events set console_error.data.text to CDP's generic "Uncaught" prefix and dropped the actual error message, which lives on the exception RemoteObject (already parsed but unused). Add a `message` field populated from the object's description (first line) or thrown value, so consumers can see what an exception said, not just that one occurred and where.
A thrown JSON null rendered as "<nil>" instead of falling back to the generic text; guard the nil case. Symbol/BigInt/NaN/Infinity throws land in unserializableValue, which was ignored and dropped the thrown value; read it after description. Adds regression tests for both.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 77185e4. Configure here.
| if i := strings.IndexByte(o.Description, '\n'); i >= 0 { | ||
| return o.Description[:i] | ||
| } | ||
| return o.Description |
There was a problem hiding this comment.
Leading newline yields empty message
Medium Severity
The exceptionMessage function returns an empty string when a CDP exception's description begins with a newline. This occurs because the first-line extraction logic misinterprets a leading newline, resulting in a blank message field in telemetry despite the generic text fallback.
Reviewed by Cursor Bugbot for commit 77185e4. Configure here.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.


Summary
Runtime.exceptionThrowntelemetry events setconsole_error.data.textto CDP's generic"Uncaught"/"Uncaught (in promise)"prefix and dropped the actual error message. The message lives on the exceptionRemoteObject(exceptionDetails.exception), which was already parsed into the struct but never read — so consumers could see that an exception fired and where (line/column/source_url/stack), but not what it said.console.error()messages were unaffected.Changes
messagetoBrowserConsoleErrorEventData(openapi.yaml + regenerated oapi).RemoteObject(description,value) and derive the message: first line ofdescription(e.g.Error: boom) → thrown value for non-Error throws → fall back to the generictext. The full stack stays instack_trace, somessageis just the name+message line.messageis present only on exceptionThrown-sourced events;textis left unchanged so the uncaught-vs-in-promise signal is preserved.Test plan
go build ./...go test ./lib/cdpmonitor/ -run TestExceptionMessagethrow new Error("...")yieldsconsole_error.data.messagecontaining the message (needs Docker + image)Note
Low Risk
Additive optional telemetry field with localized parsing logic and tests; no auth or breaking API changes.
Overview
Runtime.exceptionThrown
console_errorevents previously exposed only CDP’s generictext("Uncaught"/"Uncaught (in promise)"), so consumers could not see the real error text. This PR adds an optionalmessageonBrowserConsoleErrorEventData(OpenAPI + regeneratedoapi) and fills it in the CDP monitor when handling exceptions.exceptionMessagereads the exceptionRemoteObject: first line ofdescriptionforErrorthrows, thrown primitives viavalue, Symbol/BigInt viaunserializableValue, with fallbacks to the existingtext.textis unchanged so uncaught vs in-promise signaling stays the same; full stacks remain instack_trace.messageis set only for exceptionThrown-sourced events, notconsole.error. Unit tests cover the derivation paths.Reviewed by Cursor Bugbot for commit 77185e4. Bugbot is set up for automated code reviews on this repo. Configure here.