Skip to content

Conversation

@Mercy811
Copy link
Contributor

@Mercy811 Mercy811 commented Nov 20, 2025

Summary

https://amplitude.atlassian.net/wiki/spaces/GOV/pages/3501359165/Diagnostics+track+uncaught+SDK+errors#1.-Execution-Context-Tracking

DO NOT MERGE NOW

Let diagnostics track uncaught errors from the SDK.
#1413

Implementation:

  • Maintain a global context depth tracker
    • Increase the tracker when entering the original function and decrease the tracker when exiting
    • For async functions, put error objects in a queue pendingSDKErrors
  • Wrap main entry points: init() and process() with the DiagnosticsUncaughtError decorator which updates the tracker
  • Add an on error event handler
    • When an error happens, check if the context tracker is greater than 0 OR if it’s in pendingSDKErrors

Test server example:

GMT20251125-010300_Clip_Diagnostics.capture.uncaught.sdk.error.mp4

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

Note

Adds execution-context tracking and global web handlers to report uncaught SDK errors to Diagnostics, applies tracking to init/process, exposes decorator, and adds tests/demo; enables decorators in TS config.

  • Diagnostics (core):
    • Execution tracking: Add diagnostics-uncaught-sdk-error-global-tracker with getExecutionTracker, DiagnosticsUncaughtError decorator, and error tagging helpers.
    • Global handlers (web): Add diagnostics-uncaught-sdk-error-web-handlers to capture error and unhandledrejection and record analytics.errors.uncaught via IDiagnosticsClient.
    • Integration: Initialize error tracking in DiagnosticsClient when sampled/enabled.
    • Exports: Export DiagnosticsUncaughtError from src/index.ts.
  • Browser:
    • Decorate AmplitudeBrowser._init and process with @DiagnosticsUncaughtError to mark SDK execution.
  • Tests:
    • Add comprehensive unit tests for execution tracker and web handlers; update index.test.ts to validate new export.
  • Demo:
    • Expand test-server/diagnostics.html to demonstrate dropped events and uncaught error capture scenarios.
  • Config:
    • Enable experimentalDecorators in tsconfig.json.

Written by Cursor Bugbot for commit 5810cc2. Configure here.

@Mercy811 Mercy811 force-pushed the AMP-143673-diagnostics-uncaught-error branch from 7e0a562 to caf64f8 Compare November 20, 2025 21:37
@Mercy811
Copy link
Contributor Author

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Comment @cursor review or bugbot run to trigger another review on this PR

@Mercy811
Copy link
Contributor Author

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!


Comment @cursor review or bugbot run to trigger another review on this PR

@Mercy811
Copy link
Contributor Author

bugbot run

@Mercy811 Mercy811 marked this pull request as ready for review November 25, 2025 18:42
@Mercy811
Copy link
Contributor Author

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!


Comment @cursor review or bugbot run to trigger another review on this PR

@sojingle
Copy link
Contributor

Overall lgtm.
There is a potential issue to note: if an external callback is called synchronously by amplitude, errors that occur during this period might be treated as amplitude errors. If this situation occurs, should we treat it as by design, or can we mark such callbacks to exclude them from being recorded?

@Mercy811 Mercy811 closed this Nov 28, 2025
@Mercy811
Copy link
Contributor Author

Closing this and go with #1419 instead

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants