Page MenuHomePhabricator

Report unhandled jQuery $.Deferred errors in client side error logging
Closed, DeclinedPublic

Description

This is a feature request to enable the reporting of errors that occur in the context of a jQuery Deferred but are otherwise unhandled (do not trigger window.onerror). For example:

Example
// Simulated client error logger.
window.onerror = console.log

// Test that client error logger is invoked. (It is!)
$(() => {throw new Error})

const d = $.Deferred();
d.then(() => {
  console.log('Throwing...')
  throw new Error
  console.log('Never reached here!')
}).then(
  () => console.log('Never reached here either and the error is unhandled!')
)
d.resolve()

These sorts of errors should be reported distinctly from those that occur outside of Deferred and that are handled by window.onerror since there may be _many_ and require lots of error handler updates. Regardless, we should be able to at least easily detect quickly whether or not a deployment causes a spike in all errors.

Related: T226986

Event Timeline

@Tgr, let me know if this doesn't make sense and feel free to edit!

Thanks @Niedzielski, that matches my understanding. (Aside: testing exception throwing in the dev toolbar console can be unreliable. At least Chrome sometimes suppresses exceptions which originate in the console.)

jquery/jquery#2908 is the related upstream task (declined as there is probably no cheap non-native way for p in p.then().then().then(null, catchCallback) to tell that a catch callback exists).

AIUI this is not done and not easily doable. My old plan was T85262: Add startup script to automatically wrap asynchronous functions in try..catch which got (rightfully) rejected, and I'm not even sure if that would have been easy to apply to promises. Without something drastic like switching from jQuery promises to native promises, I'm not sure how it could be done.

That was my assessment too. We have tasks for switching to native Promises but I don't see this task as actionable in any way. Shall we decline ?

Done; if anyone disagrees or has an idea on how to do it, please feel free to reopen.

We have tasks for switching to native Promises

FWIW that's T237688: Raise MW JS requirement to include ES6 Promise (drop IE11, Safari 5-6, and Android 4.1-4.3).

...or maybe this should be kept open with the native promises task being a blocker? When/if that happens, this will be an actionable (and fairly easy) request.