Page MenuHomePhabricator

jQuery 3 catches all exceptions within promises
Closed, ResolvedPublic

Description

I understand this is part of the spec, but as a developer I want the browser to pause on uncaught exceptions. Anything else makes debugging a lot harder, so there should be a flag to disable this. In VE in particular a large percentage of code is run inside a promise.

Event Timeline

Esanders created this task.Jun 16 2017, 6:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 16 2017, 6:47 PM

jQuery upgrade notes suggest https://github.com/dmethvin/jquery-deferred-reporter – although I am a bit confused as it also claims that exceptions should already be logged without a backtrace even without that plugin – but I am not seeing anything in the console.

Krinkle raised the priority of this task from Normal to High.Jun 21 2017, 6:56 PM

So the issue is when using .then() without .catch() that logs/rethrows the error. Then there is no issue when using done() and fail(). The jquery-deferred-reporter seems useless as I don't see any difference in behavior with it (or I don't know how to use it properly.

Krinkle added a comment.EditedJun 22 2017, 10:41 PM

@Nikerabbit I wrote about the promise changes at T124742#3373541.

Default

The main hook for handling exceptions from promises is jQuery.Deferred.exceptionHook. It is called when an exception is caught from a then-callback.

As documented at https://jquery.com/upgrade-guide/3.0/#deferred:

jQuery logs a message to the console when it is inside a Deferred and a JavaScript exception occurs. These messages take the form jQuery.Deferred exception: (error message). If you do not want any console output on these exceptions, set jQuery.Deferred.exceptionHook to undefined. [...] use the jquery-deferred-reporter plugin during development to obtain stack traces.

jQuery 3
var d = $.Deferred().resolve( 'value' );
d.then( function ( val ) {
  return new mw.unknown.Bar( val );
} );
Console
jQuery.Deferred exception: Cannot read property 'Bar' of undefined
    at <anonymous>:3:24
    at mightThrow (/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&...:49:598)
    at process

Two problems:

  • jQuery Migrate breaks the logger.
  • The default logger filters out custom errors.

jQuery Migrate

Because jQuery Migrate restores some other internal behaviours from jQuery 1 in jQuery 3, it has to replace the jQuery.Deferred function object. In doing so, it naturally dereferences the default jQuery.Deferred.exceptionHook. This is a bug that will be fixed upstream in the next release. See https://github.com/jquery/jquery-migrate/pull/262

Until then, you can work-around it by running the following from the console (which restores the default handler from jQuery 3)

jquery-exceptionHook.js

jQuery.Deferred.exceptionHook = function( error, stack ) {
	if ( /^(Eval|Internal|Range|Reference|Syntax|Type|URI)Error$/.test( error.name ) ) {
		console.warn( "jQuery.Deferred exception: " + error.message, error.stack, stack ); } };

Custom errors

If you're still not getting a log message, then it is probably because the code in question explicitly throws a custom exception, like the below. These are not logged by default.

var d = $.Deferred().resolve( 'value' );
d.then( function ( val ) {
  throw new Error( 'Aye' );
} );

The default handler (source code) explicitly logs run-time errors from JavaScript or the browser, but ignores custom made exceptions that are explicitly thrown by user code.

Think about it, usually if you throw an exception somewhere, you catch it. It only goes to the console if it's an uncaught one. In case of promises, we can't know at run-time whether it will be caught or not. Many applications use exceptions internally. We could override exceptionHook in MediaWiki to log unconditionally, but will slow down exception, and cause a fair amount of log spam by also logging all errors that are thrown internally and then normally dealt with.

$.ready

Note that there is a special "exception" for the common case of $.ready (which isn't a promise typically chained on or returned from a method), exceptions in those handlers are effectively logged unconditionally because jQuery rethrows those asynchronously for native detection by the browser (console, onerror etc.). Background: https://github.com/jquery/jquery/issues/3174

I suggest we start by updating jQuery Migrate to at least re-enable the default logger.

Logging all internal exceptions by default seems overkill. Much like how browsers don't pause on (caught) exceptions by default. You can, however, enable this at run-time with the following snippet:

Verbose
jQuery.Deferred.exceptionHook = function (error, stack) { console.warn('jQuery.Deferred exception: ' + error.message, error.stack, stack);}};

Change 360999 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Backport jquery-migrate.js patch for exceptionHook

https://gerrit.wikimedia.org/r/360999

Change 360999 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Backport jquery-migrate.js patch for exceptionHook

https://gerrit.wikimedia.org/r/360999

Is it unavoidable that these exceptions will all now be caught? A logged stack trace is considerably less useful than an uncaught exception that pauses the debugger.

@Esanders Once you know where it's coming from you can set a breakpoint or use debugger;. Alternatively, you can do it ahead of time by using the hook jQuery.Deferred.exceptionHook, which allows this kind of altering. It's not perfect, but it's the best we can do given these are not global exceptions and also not native promises (for which Chrome has debugging as well).

jQuery.Deferred.exceptionHook = function () { throw 'debugger'; }; // Pause on uncaught exceptions

or:

jQuery.Deferred.exceptionHook = function () { debugger; }; // Directly

And then run:

$.Deferred().resolve().then(function () { throw new Error('x'); });

...unless the exception is on the thousandth iteration of a loop, in which case you need to make the breakpoint conditional, and then you need to work out what the condition is.

...unless the exception is on the thousandth iteration of a loop, in which case you need to make the breakpoint conditional, and then you need to work out what the condition is.

Setting a debugger breakpoint using jQuery.Deferred.exceptionHook will break on any uncaught exception without needing to know the condition. And from there you can jump to any other concurrent point upward in the stack, and inspect their local scopes. However, it's possible that jQuery's catch is too high in the stack (e.g. the local state that caused it may've fallen out of scope by then). In that case, a try/catch closer to the throw would work better.

for (let i = 0; i < 1000; i++) {
try{
    if (Math.random() < 0.01) {
        throw new Error();
    }
}catch(e){ debugger; } // paused when e.g. 'i' is 243
}

This can become tedious if the problem may come from multiple areas. You can also try the inverse and start from the throw statement instead. The breakpoint doesn't have to share any lexical scope with the place you want to debug (e.g. conditions of a loop). Below an exception is thrown from one method, but called from another (and the functions are not nested). You can step to any concurrent point higher in the stack. Their scopes, too, are inspectable.

function useless() {
  var x = Math.random();
  if (x < 0.5) { debugger; // or point-to-click run time breakpoint on the line below
    throw new Error('x');
  }
}

function other() {
  var i = 1000;
  while (i--) {
    useless();
  }
}
other();
Krinkle closed this task as Resolved.Jul 8 2017, 3:39 AM
Krinkle claimed this task.
Krinkle removed a project: Patch-For-Review.
Krinkle moved this task from Accepted: Bugs to Assigned on the MediaWiki-ResourceLoader board.
Krinkle added a project: Performance-Team.