Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Show LESS errors in console, more thoroughly #9557

Merged
merged 1 commit into from
Dec 5, 2014
Merged

Show LESS errors in console, more thoroughly #9557

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/utils/ExtensionUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ define(function (require, exports, module) {
try {
result.resolve(tree.toCSS());
} catch (toCSSError) {
console.error(toCSSError.filename + ":" + toCSSError.line + " " + toCSSError.message);
result.reject(toCSSError);
}
}
Expand Down Expand Up @@ -220,6 +219,16 @@ define(function (require, exports, module) {
})
.fail(result.reject);

// Summarize error info to console for easier debugging
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);
}
});

Copy link
Contributor

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".

Copy link
Member Author

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.

Copy link
Contributor

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.

return result.promise();
}

Expand Down