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
Conversation
This file contains 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
|
|
||
| function installGlobalHandler () | ||
| { | ||
| if (window.onerror === traceKitWindowOnError) { |
There was a problem hiding this comment.
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?
|
Sure, I'll do so tomorrow. |
|
Actually- I just patched it. |
|
Seems good, thanks. I'll close this then. |
|
Merged to master. Thanks for the patch |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This changes TraceKit to only install a window.onerror handler if we get a subscriber.
Even though the onerror handler is always necessary when catching exceptions for IE, there's no need for the handler if there are no exception handlers, since the exception would be ignored anyway.
Catching exceptions in development mode can be very annoying and can mess up debugging by changing backtraces to point to the last point where an exception has been rethrown, for example. For this reason, it's often easiest to not have any global exception handlers while debugging.
The way TraceKit was set up, this basically meant you were forced not to load in TraceKit code during debugging. This also then means that you'll get a lot of extra code in your production builds that you haven't tested in development.
With this change, TraceKit can still be loaded in development builds. If no subscriber is registered for handling exceptions, no global error handler will be installed, so debug builds can continue to work as normal.
Refers to #21 and #14.