Page MenuHomePhabricator

Catch uncaught exceptions with Sentry
Closed, ResolvedPublic1 Estimated Story Points

Description

When raven.js is installed, it automatically catches and reports uncaught exceptions via window.onerror. We need a way to catch errors which happen before raven.js is loaded, though - in part because some modules might be loaded before Sentry is, and in debug mode window.onerror is the only way to catch module initialization errors; and in part because we might want to lazy-load raven.js.

We can use the same mw.track mechanism that is used to catch async callback errors; the difficulty is that there is no way to make raven.js not handle window.onerror, so we must be careful not to report errors twice.

Event Timeline

Tgr raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added a project: Sentry.
Tgr subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Tgr set Security to None.

I'm rolling these Sentry changesets into the next sprint for testing purposes, because right now I can't even test locally because of T90701

I can't seem to make this work, on Firefox. I've added an uncaught exception in Media Viewer when clicking on a specific button, it shows up in the console but not in raven.

Maybe this depends on the core changes that are still in review?

Change 192755 abandoned by Gergő Tisza:
Report uncaught errors via mw.track

Reason:
Abandoning. Will redo in another patch which does not depend on async wrapping as window.onerror-based error logging seems to be an easier win.

https://gerrit.wikimedia.org/r/192755

Change 199557 had a related patch set uploaded (by Gergő Tisza):
Report uncaught errors via mw.track

https://gerrit.wikimedia.org/r/199557

Reopening - removing the default handler is crappy behavior. Sentry should just unregister itself from mw.track instead.
Also, the core patch for this is not merged yet.

Tgr triaged this task as High priority.Mar 25 2015, 10:25 PM

@Tgr: All patches in Gerrit merged or abandoned; is there more to do here?

No reply from @Tgr hence assuming this has been fixed