Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

dojo/on: added on.handleError for addListener #1

Merged
merged 1 commit into from

3 participants

@miksago

No description provided.

@TheAndySmith

On the one hand I'm not sure that this is very useful since it would seem to be easy to make listeners catch their errors. A higher-order function to augment listeners, or a drop-in replacement for on() that always augments listeners, would be easy ways to do that.

On the other hand if there was to be a global error handler for on(), some ideas for making it more useful:

  • Maybe an event-based system would be a nicer way to set the error handler, eg on(on, "error", handler), although I guess the error handling shouldn't apply to itself, and I think this means you'd have to augment the listener regardless of whether an error handler is in use.

  • More information could be useful in the error handler: the event emitter, event type, maybe the listener, and maybe the handle returned by on() or some other reference to this particular event handling installation (in particular so the error handler can remove the listener).

  • Should the error be re-thrown after being passed to the error handler? I suppose the error handler can throw it if it wants, so not doing it from on() is more flexible.

  • As it stands the error handler is bypassed if the event emitter has an on() method that doesn't use dojo/on.

@novemberborn

We're improving the reporting of client-side errors, however due to cross-domain restrictions on error objects and the script origin (CDN origin versus page origin), letting errors from event handlers bubble to window.onerror masks any useful information. If we can capture the error in a script served from the CDN origin, and also report from a script served from the CDN origin, we can capture all information.

For nearly all Dojo code this is the lowest level of event binding.

on.js
@@ -105,7 +105,17 @@ define(["./has!dom-addeventlistener?:./aspect", "./_base/kernel", "./has"], func
return addListener(target, type, listener, dontFix, matchesTarget);
};
var touchEvents = /^touch/;
- function addListener(target, type, listener, dontFix, matchesTarget){
+ function addListener(target, type, _listener, dontFix, matchesTarget){
+ var listener = _listener;
+ if(typeof on["handleError"] === "function"){
@novemberborn Owner

Why the [""]?

@miksago
miksago added a note

I could just do if(on.handleError) I think...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@novemberborn

As it stands the error handler is bypassed if the event emitter has an on() method that doesn't use dojo/on

This is supposed to only catch errors thrown by DOM event handlers, so that's OK.

More information could be useful in the error handler: the event emitter, event type, maybe the listener, and maybe the handle returned by on() or some other reference to this particular event handling installation (in particular so the error handler can remove the listener).

Possibly, but we should trial this internally first.

@miksago

Maybe an event-based system would be a nicer way to set the error handler, eg on(on, "error", handler)

We don't really want there to be multiple handlers for this, really..

@miksago miksago merged commit d366e2a into master
@miksago miksago deleted the capture-errors-in-on branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 14, 2013
  1. @miksago
This page is out of date. Refresh to see the latest.
Showing with 11 additions and 1 deletion.
  1. +11 −1 on.js
View
12 on.js
@@ -105,7 +105,17 @@ define(["./has!dom-addeventlistener?:./aspect", "./_base/kernel", "./has"], func
return addListener(target, type, listener, dontFix, matchesTarget);
};
var touchEvents = /^touch/;
- function addListener(target, type, listener, dontFix, matchesTarget){
+ function addListener(target, type, _listener, dontFix, matchesTarget){
+ var listener = _listener;
+ if(typeof on.handleError === "function"){
+ listener = function(){
+ try{
+ return _listener.apply(this, arguments);
+ }catch(err){
+ on.handleError(err);
+ }
+ };
+ }
// event delegation:
var selector = type.match(/(.*):(.*)/);
// if we have a selector:event, the last one is interpreted as an event, and we use event delegation
Something went wrong with that request. Please try again.