Skip to content

Commit

Permalink
Deferred: Rename getStackHook to getErrorHook (3.x version)
Browse files Browse the repository at this point in the history
Rename `jQuery.Deferred.getStackHook` to `jQuery.Deferred.getErrorHook`
to indicate passing an error instance is usually a better choice - it
works with source maps while a raw stack generally does not.

In jQuery `3.7.0`, we'll keep both names, marking the old one as
deprecated. In jQuery `4.0.0` we'll just keep the new one. This
change implements the `3.7.0` version; PR gh-5211 implements
the `4.0.0` one.

Fixes gh-5201
Closes gh-5212
Ref gh-5211
  • Loading branch information
mgol authored Mar 14, 2023
1 parent 3b7bf19 commit cca7118
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 11 deletions.
15 changes: 11 additions & 4 deletions src/deferred.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ jQuery.extend( {

if ( jQuery.Deferred.exceptionHook ) {
jQuery.Deferred.exceptionHook( e,
process.stackTrace );
process.error );
}

// Support: Promises/A+ section 2.3.3.3.4.1
Expand Down Expand Up @@ -222,10 +222,17 @@ jQuery.extend( {
process();
} else {

// Call an optional hook to record the stack, in case of exception
// Call an optional hook to record the error, in case of exception
// since it's otherwise lost when execution goes async
if ( jQuery.Deferred.getStackHook ) {
process.stackTrace = jQuery.Deferred.getStackHook();
if ( jQuery.Deferred.getErrorHook ) {
process.error = jQuery.Deferred.getErrorHook();

// The deprecated alias of the above. While the name suggests
// returning the stack, not an error instance, jQuery just passes
// it directly to `console.warn` so both will work; an instance
// just better cooperates with source maps.
} else if ( jQuery.Deferred.getStackHook ) {
process.error = jQuery.Deferred.getStackHook();
}
window.setTimeout( process );
}
Expand Down
8 changes: 6 additions & 2 deletions src/deferred/exceptionHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ define( [
// warn about them ASAP rather than swallowing them by default.
var rerrorNames = /^(Eval|Internal|Range|Reference|Syntax|Type|URI)Error$/;

jQuery.Deferred.exceptionHook = function( error, stack ) {
// If `jQuery.Deferred.getErrorHook` is defined, `asyncError` is an error
// captured before the async barrier to get the original error cause
// which may otherwise be hidden.
jQuery.Deferred.exceptionHook = function( error, asyncError ) {

// Support: IE 8 - 9 only
// Console exists when dev tools are open, which can happen at any time
if ( window.console && window.console.warn && error && rerrorNames.test( error.name ) ) {
window.console.warn( "jQuery.Deferred exception: " + error.message, error.stack, stack );
window.console.warn( "jQuery.Deferred exception: " + error.message,
error.stack, asyncError );
}
};

Expand Down
10 changes: 5 additions & 5 deletions test/unit/deferred.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,13 +628,13 @@ QUnit[ window.console ? "test" : "skip" ]( "jQuery.Deferred.exceptionHook with s
defer = jQuery.Deferred(),
oldWarn = window.console.warn;

jQuery.Deferred.getStackHook = function() {
jQuery.Deferred.getErrorHook = function() {

// Default exceptionHook assumes the stack is in a form console.warn can log,
// but a custom getStackHook+exceptionHook pair could save a raw form and
// but a custom getErrorHook+exceptionHook pair could save a raw form and
// format it to a string only when an exception actually occurs.
// For the unit test we just ensure the plumbing works.
return "NO STACK FOR YOU";
return "NO ERROR FOR YOU";
};

window.console.warn = function() {
Expand All @@ -656,13 +656,13 @@ QUnit[ window.console ? "test" : "skip" ]( "jQuery.Deferred.exceptionHook with s
} else {
assert.ok( /cough_up_hairball/.test( msg ), "Function mentioned: " + msg );
}
assert.ok( /NO STACK FOR YOU/.test( msg ), "Stack trace included: " + msg );
assert.ok( /NO ERROR FOR YOU/.test( msg ), "Error included: " + msg );
};
defer.then( function() {
jQuery.cough_up_hairball();
} ).then( null, function( ) {
window.console.warn = oldWarn;
delete jQuery.Deferred.getStackHook;
delete jQuery.Deferred.getErrorHook;
done();
} );

Expand Down
36 changes: 36 additions & 0 deletions test/unit/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -661,4 +661,40 @@ QUnit.test( "trim", function( assert ) {
assert.equal( jQuery.trim( "\uFEFF \xA0! | \uFEFF" ), "! |", "leading/trailing should be trimmed" );
} );

if ( includesModule( "deferred" ) ) {
QUnit.test( "jQuery.Deferred.exceptionHook with stack hooks", function( assert ) {

assert.expect( 2 );

var done = assert.async(),
defer = jQuery.Deferred(),
oldWarn = window.console.warn;

jQuery.Deferred.getStackHook = function() {

// Default exceptionHook assumes the stack is in a form console.warn can log,
// but a custom getStackHook+exceptionHook pair could save a raw form and
// format it to a string only when an exception actually occurs.
// For the unit test we just ensure the plumbing works.
return "NO STACK FOR YOU";
};

window.console.warn = function() {
var msg = Array.prototype.join.call( arguments, " " );
assert.ok( /cough_up_hairball/.test( msg ), "Function mentioned: " + msg );
assert.ok( /NO STACK FOR YOU/.test( msg ), "Stack trace included: " + msg );
};

defer.then( function() {
jQuery.cough_up_hairball();
} ).then( null, function() {
window.console.warn = oldWarn;
delete jQuery.Deferred.getStackHook;
done();
} );

defer.resolve();
} );
}

}

0 comments on commit cca7118

Please sign in to comment.