Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add Events to Raven #84

Merged
merged 2 commits into from Apr 18, 2013
Merged

RFC: Add Events to Raven #84

merged 2 commits into from Apr 18, 2013

Conversation

bisrael
Copy link
Contributor

@bisrael bisrael commented Mar 13, 2013

Please do not accept this pull request yet

This adds several events:

document.addEventListener('ravenHandle', myCallback);
document.removeEventListener('ravenHandle', myCallback);

Right now, I'm triggering three events:

  • ravenHandle: triggers as Raven gets called from TraceKit to handle an error
    • properties added to event object: stackInfo, options
  • ravenSuccess: triggers when Raven has successfully reported an error to sentry
    • properties added to event object: data, src
  • ravenFailure: triggers when Raven tried to report an error, but got back an bad response code.
    • properties added to event object: data, src

Discussion Points:

  • Callback Names
  • Callback Arguments (I definitely do not like what i'm passing now)
  • Eventing Mechanism (is it too complex?)

Relevant Issues: #81 and #83

Edit: Updated to match current commits

@mattrobenolt
Copy link
Contributor

First impression, the #1 concern is bloat. I'd like to leverage existing browser events if possible. I don't know too much about real events off the top of my head, but if it requires this much bloat to achieve it in a relatively compatible fashion, I'll just stick with callbacks. One of the goals of Raven is to be as light as possible since we don't want to have to promote deferred loading.

@bisrael
Copy link
Contributor Author

bisrael commented Mar 14, 2013

Bloat is definitely a huge concern. I'm redoing this with DOM Events and we'll take a look at how that works.

@bisrael
Copy link
Contributor Author

bisrael commented Mar 14, 2013

I updated the code to fire native events on the document.

@bisrael
Copy link
Contributor Author

bisrael commented Mar 14, 2013

I also updated the top post with how it works now

@bisrael
Copy link
Contributor Author

bisrael commented Mar 15, 2013

@mattrobenolt - have you had a chance to look over the changes?

@mattrobenolt
Copy link
Contributor

@bisrael, I have not. I'm currently at PyCon, so I'll have to take a closer look after this weekend.

@mattrobenolt
Copy link
Contributor

This looks good to me. Sorry it took so long. :)

mattrobenolt added a commit that referenced this pull request Apr 18, 2013
RFC: Add Events to Raven
@mattrobenolt mattrobenolt merged commit 9b2887b into getsentry:master Apr 18, 2013
@vperron
Copy link
Contributor

vperron commented Jun 5, 2015

Hello, I know the PR is closed and all, but I have stumbled upon libraries that use those messages and have not found much documentation about it...
Did I miss something or would it be nice to actually document the feature ?

kamilogorek pushed a commit that referenced this pull request Jun 12, 2018
Fix rare recursion condition in patchGlobal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants