-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Show LESS errors in console, more thoroughly #9557
Conversation
result.fail(function (error) { | ||
console.error("[Extension] Unable to load stylesheet " + path, error); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be more helpful to split this across the 2 fail handlers so user would get separate messages for "error loading file" and "error parsing LESS file".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@redmunds The different error cases are already distinguishable because the error objects is included in the log message -- though the message isn't super clear in the case of a loadFile()
failure due to the way $.get()
passes error objects back.
We'd actually need 3 different fail handlers to cover all the cases, if we split the handler, since there are 3 different places where reject() could be called. How about just detecting the two types of error results in a single handler, like this:
result.fail(function (error, textStatus, httpError) {
if (error.readyState !== undefined) {
// If first arg is a jQXHR object, the real error info is in the next two args
console.error("[Extension] Unable to read stylesheet " + path + ":", textStatus, httpError);
} else {
console.error("[Extension] Unable to process stylesheet " + path, error);
}
});
That improves the clarity of the message shown for loading errors, yet still guarantees we'll always log something in every failure case that could arise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that works for me.
@peterflynn Done with review |
…o the console (not just the output stage). Also covers cases where the file doesn't exist, for both CSS & LESS.
ceea406
to
c625d02
Compare
@redmunds Cool, thanks -- changes pushed. (I squashed so it's still a single commit). |
Merging. |
Show LESS errors in console, more thoroughly
Refinement of PR #8736: also log LESS errors from the parsing stage to the console (not just the output stage). Also covers cases where the file doesn't exist, for both CSS & LESS.