Currently we're using alert to show ajax errors. This is not a good way to deal with this.

We should be making our own Error type and throwing that to show when there is something that broke down. using alert or even console.log is a poor substitution when throw is really what we mean and want to do.

function AJAXError(message) {
  this.name = 'AJAXError';
  this.message = message || 'Default txt';
}
AJAXError.prototype = new Error();

This is the amount of code we need to throw proper errors that will show up with a stacktrace and everything we need to debug it.
Files: 
CommentFileSizeAuthor
#15 interdiff.txt690 bytesnod_
#13 core-js-throw-ajaxerror-1606362-13.patch2.92 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 55,597 pass(es).
[ View ]
#11 ajax-error-1606362-11.patch2.81 KBseutje
PASSED: [[SimpleTest]]: [MySQL] 46,442 pass(es).
[ View ]
#11 ajax-error-1606362-11-interdiff.txt1016 bytesseutje
#4 ajax-error-1606362-4.patch2.81 KBAngry Dan
PASSED: [[SimpleTest]]: [MySQL] 41,478 pass(es).
[ View ]

Comments

window.alert isn't a good way to deal with anything, except intentionally halting execution and possibly breaking a thing or 2 in the process.

Having real errors being thrown would not only keep certain interactions/animations from being halted (and evidentally breaking), but it would also allow us to make the error handling pluggable (e.g. a module could define an error handler that display it in a fancy modal, another could define a handler that posts it back to the server, ...)

Awesome idea! I also agree, window.alert() is rather annoying.

StatusFileSize
new2.81 KB
PASSED: [[SimpleTest]]: [MySQL] 41,478 pass(es).
[ View ]

This patch replaces Drupal.ajaxError with an object that extends Error. I've also removed the usages of alert(Drupal.ajaxError()) and replaced them with thrown errors.

On it's own this would probably mean that there isn't enough feedback to the user in most cases, so I think we need to pursue some of the issues in #1419652: [Meta] javascript logging and error reporting, specifically #1232416: Drupal alerts "An AJAX HTTP request terminated abnormally" during normal site operation, confusing site visitors/editors before this can stand up.

Status:Active» Needs review

Hey thanks for working on it :)

Any particular reason you went with using call on the constructor instead of prototypal inheritence? Typically I see throw new Error('my message'); when using Error, not so much this way of doing things.

I must admit this is the first time that I've looked at JS for a couple few months now. I would like to see subtypes of the Error function for various errors in Drupal JS, just like you might want to do with the Exception class in PHP.

I was struggling to find the best way of subclassing Error. Are you suggesting that you wouldn't have done that and instead used descriptive error messages instead? I'm interested how would you have implemented this? I'm always looking for ways to improve!

Read the issue summary or look at drupal.js :) That's how I'd have done it.

(edit) and considering classes don't exist you can't really subclass it, you can inherit from it though.

#7 can you create a follow up issues for the subtypes?

I don't think we'd have that many, we can list them here.

there is DrupalBehaviorError already and we'd be creating a DrupalAjaxError for ajax things and maybe one other, not sure what for yet but I'm not seeing tens of error types defined by Drupal (well, I hope at least).

StatusFileSize
new1016 bytes
new2.81 KB
PASSED: [[SimpleTest]]: [MySQL] 46,442 pass(es).
[ View ]

Removed some whitespace and added a semicolon that was removed.

Hi all,

I haven't looked at this in a while now, but I thought I would link this article - https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Obj... which is where I got the methodology for extending Error in the way I did.

What else needs doing on this? I think it's probably ready to go in now, then we can move stuff around later/create new error types.

StatusFileSize
new2.92 KB
PASSED: [[SimpleTest]]: [MySQL] 55,597 pass(es).
[ View ]

Removed the Error.call(), it's not needed, just need to set this.messages and it'll do the job.

Side effect of this patch is that there is no way to see an error if you don't have a JS console open somewhere. On way to address that is to use the future #77245: A place for JavaScript status messages. There is no way we'll be able to fix everything in one patch, this one is the first step to fix the really really annoying #1232416: Drupal alerts "An AJAX HTTP request terminated abnormally" during normal site operation, confusing site visitors/editors issue.

Status:Needs review» Reviewed & tested by the community

Same code as #11 and that worked.

StatusFileSize
new690 bytes

interdiff

Looks fine to me.

Title:Use throw when error occursUse throw when an ajax error occurs and extend JavaScript's Error object

Improving title

Status:Reviewed & tested by the community» Fixed

Committed 1e40d90 and pushed to 8.x. Thanks!

What's the end-user experience for this now when a JS error is thrown? Will it silently fail unless you have a JS console open? If so, are we sure that's preferable?

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.