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

Allow non-functions to be passed to Rollbar.wrap #17

Merged
merged 1 commit into from
Mar 7, 2014
Merged

Allow non-functions to be passed to Rollbar.wrap #17

merged 1 commit into from
Mar 7, 2014

Conversation

kmontag
Copy link
Contributor

@kmontag kmontag commented Mar 4, 2014

Rollbar.wrap currently returns a function that errors out immediately (with "has no method 'apply'") when passed a non-function. This causes errors when, for example, addEventListener is called with an object implementing EventListener (https://developer.mozilla.org/en-US/docs/Web/API/EventListener) rather than a plain function.

@brianr
Copy link
Member

brianr commented Mar 4, 2014

Thanks @kmontag! This looks good to me, will wait for the travis build to pass and then merge once @coryvirok can get some eyes on it.

To take this a step further in the EventListener case, do you think it would make sense to wrap the passed-in object's handleEvent method? (assuming the property exists and is a function)

@kmontag
Copy link
Contributor Author

kmontag commented Mar 4, 2014

@brianr I thought about that, but was hesitant to modify the handleEvent method on the object directly. And creating a copy of the object to give to addEventListener would mean the copy and the original object could get out of sync.

Maybe it's enough to do something like https://github.com/kmontag/rollbar.js/commit/a193d0517a8eb42f790a2d780dd9f64f6adb7bad ? But I feel like there may be some edge cases I'm missing...

@coryvirok
Copy link
Contributor

This looks good. I'll create a new release shortly.

coryvirok added a commit that referenced this pull request Mar 7, 2014
Allow non-functions to be passed to Rollbar.wrap
@coryvirok coryvirok merged commit b0d3613 into rollbar:master Mar 7, 2014
@kmontag kmontag deleted the wrap-nonfunctions branch March 7, 2014 22:59
mudetroit pushed a commit that referenced this pull request Mar 14, 2024
Allow non-functions to be passed to Rollbar.wrap
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.

3 participants