magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.48k stars 9.29k forks source link

Suggestion for improved error reporting for require.js loading problems #36761

Open hostep opened 1 year ago

hostep commented 1 year ago

Summary

Hi there

I recently ran into a hard to figure out problem after a Magento upgrade, where one of the custom scripts we had could not be loaded through require.js. It showed an error like this in the browser's console:

Failed to load the "{customComponent}" component.

This error is pretty vague and I had a hard time figuring out what was the problem, so I eventually started looking if there was a way to expose more details about the error. And that lead me to the app/code/Magento/Ui/view/base/web/js/core/renderer/layout.js file. I noticed that I was able to improve the output by outputting the error itself to the console instead of only a generic message (see below for proposed solution).

Once I added that, it was immediately clear what the error was, it wasn't a problem in the {customComponent} but in one of its dependencies, and it was eventually an easy fix. But the time spend on trying to locate the error was like 5 times the time it took to fix the problem. So this extra error output can really help to find hard to diagnose javascript problems.

Would Magento be open to such an improvement? I could send in a PR if I know that it would be accepted.

Examples

Not much time at the moment to figure out an example case, maybe I'll find some time next week or even later.

Proposed solution

diff --git a/app/code/Magento/Ui/view/base/web/js/core/renderer/layout.js b/app/code/Magento/Ui/view/base/web/js/core/renderer/layout.js
index 5240fe55f6a..d8c6e7e3088 100644
--- a/app/code/Magento/Ui/view/base/web/js/core/renderer/layout.js
+++ b/app/code/Magento/Ui/view/base/web/js/core/renderer/layout.js
@@ -118,7 +118,8 @@ define([
                 component: node.component
             });
             loaded.resolve(node, constr);
-        }, function () {
+        }, function (e) {
+            consoleLogger.error(e);
             consoleLogger.error('componentLoadingFail', {
                 component: node.component
             });

Release note

We've improved the output of the error when require.js can't load a certain component so it's easier for developers to figure out the cause of the error.

Triage and priority

m2-assistant[bot] commented 1 year ago

Hi @hostep. Thank you for your report. To speed up processing of this issue, make sure that you provided the following information:

Make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, review the Magento Contributor Assistant documentation.

Add a comment to assign the issue: @magento I am working on this

To learn more about issue processing workflow, refer to the Code Contributions.


:clock10: You can find the schedule on the Magento Community Calendar page.

:telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

:pencil2: Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

mrtuvn commented 1 year ago

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Ui/view/base/web/js/lib/logger/console-logger.js Out of curiousity do we actually need log always load at all page? Should have config allow we on/off load this or exclude it in production ? Not sure this log helpful if we can use default log javascript native instead ?

hostep commented 1 year ago

Hmm, that seems to be a whole other discussion, maybe you should open a new issue for that? 🙂

ludcl commented 4 months ago

Thanks @hostep! I saw this more than 1 year later and it's still very useful. 😎 I was getting crazy trying to debug reading the useless default log messages. After using your patch, it took me a couple of minutes to fix all the issues.

anievolve commented 1 month ago

Thanks @hostep 2 days of debugging solved in 5 minutes

hostep commented 1 month ago

Okay, great to hear these success stories, I'll try to remember to send in a pull request with this change, maybe it will be included in the next Magento version then ...