Skip to content
This repository has been archived by the owner on Jan 21, 2024. It is now read-only.

Only install global error handler if we have at least one subscriber #37

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions tracekit.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ TraceKit.report = (function reportModuleWrapper() {
* @param {Function} handler
*/
function subscribe(handler) {
installGlobalHandler();
handlers.push(handler);
}

Expand Down Expand Up @@ -145,7 +146,7 @@ TraceKit.report = (function reportModuleWrapper() {
}
}

var _oldOnerrorHandler = window.onerror;
var _oldOnerrorHandler;

/**
* Ensures all global unhandled exceptions are recorded.
Expand All @@ -155,7 +156,7 @@ TraceKit.report = (function reportModuleWrapper() {
* @param {(number|string)} lineNo The line number at which the error
* occurred.
*/
window.onerror = function traceKitWindowOnError(message, url, lineNo) {
function traceKitWindowOnError(message, url, lineNo) {
var stack = null;

if (lastExceptionStack) {
Expand Down Expand Up @@ -186,7 +187,16 @@ TraceKit.report = (function reportModuleWrapper() {
}

return false;
};
}

function installGlobalHandler ()
{
if (window.onerror === traceKitWindowOnError) {
Copy link
Owner

Choose a reason for hiding this comment

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

I think an internal flag would be better here.

With multiple libraries attaching to window.onerror, this can end up badly.
Do you mind adding a flag? Like- _isOnerrorHandlerInstalled?

return;
}
_oldOnerrorHandler = window.onerror;
window.onerror = traceKitWindowOnError;
}

/**
* Reports an unhandled Error to TraceKit.
Expand Down