prototypejs / prototype

Prototype JavaScript framework
http://prototypejs.org/
Other
3.54k stars 639 forks source link

Add default exception handler to Ajax.Base #302

Closed Gargaj closed 7 years ago

Gargaj commented 8 years ago

If this isn't specified, any exception that is generated inside an Ajax call is swallowed, which I think is mostly an undesired effect since execution just stops without any feedback in e.g. the console or the user's outer exception handler.

savetheclocktower commented 8 years ago

Marking this as a bug, but I'll have to decide on how best to handle this. Adopting this PR would mean a change to existing default behavior.

Gargaj commented 8 years ago

I would argue that any site that relies on a framework to swallow exceptions should be forced to do it manually. If your site is continually throwing exceptions, the framework shouldn't be holding them back - relying on it is just bad behaviour that should not be condoned.

If anything, having this in a future version's patch notes would encourage site owners to go through their existing Ajax and check if their code is solid when it comes to this.

Gargaj commented 7 years ago

@savetheclocktower Can has 1.7.4 plees? See my argument above about why this would be necessary for a better web.

savetheclocktower commented 7 years ago

How about this as the default handler:

function(request, exception) {
  setTimeout(function() { throw exception; }, 0);
}

My concern is that if anyone is relying on current behavior, upgrading will cause their code execution to halt in places it didn't before. At least this way the only behavior change is that exceptions would show up in the console where they didn't before.

Gargaj commented 7 years ago

If they're relying on current behaviour, their code will halt anyway, won't it? Just because they don't see the Exception doesn't mean it doesn't fire and kill execution.

jwestbrook commented 7 years ago

Idea : instead of throwing exception - use console.warn or console.error if it exists? It keeps the current behavior but notifies the dev that something went wrong

savetheclocktower commented 7 years ago

My point is that an asynchronous throw will halt only the stuff that’s happening in that scheduled unit. But then I suppose my suggestion is only an improvement for synchronous requests, which are dumb anyway.