#14707 closed defect (fixed)
[regression] Loader silently fails by default when modules 404
Reported by: | Colin Snover | Owned by: | Rawld Gill |
---|---|---|---|
Priority: | blocker | Milestone: | 1.7.2 |
Component: | Loader | Version: | 1.7.1 |
Keywords: | Cc: | ||
Blocked By: | #14750 | Blocking: |
Description (last modified by )
When someone typos a module name and it 404s there is *no* indication that this has happened (except in Chrome which reports network errors to the console by default) unless you watch the Net tab for 404s or set Firebug to report network errors to the console (it does not by default AFAIK). Dojo 1.6 had a default timeout of 15 seconds. Dojo 1.7 has a default timeout of 0 (disabled).
Change History (18)
comment:1 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Priority: | undecided → blocker |
Summary: | Loader silently fails by default when modules 404 → [regression] Loader silently fails by default when modules 404 |
comment:2 Changed 9 years ago by
Priority: | blocker → low |
---|---|
Status: | new → assigned |
Type: | defect → enhancement |
comment:3 Changed 9 years ago by
While I can somewhat see the "you're not going to wait around 15s" argument, I think we all might be surprised at just how effective it might be. Between gawking at the screen like "wtf", going back to your editor to try to check something, banging your head on the keyboard, etc., by the time you look at your browser again, you may very well notice the error popped up.
Personally though, I'd approach this more from the following perspectives:
- If 1.6 had a 15s timeout, what's the harm in continuing to provide it?
- I agree that never hearing an error out of this (at least without heading over to the net tab) is harmful to devs in the long run. Critical failures should be loud so that they can be identified as easily as possible.
comment:4 Changed 9 years ago by
Type: | enhancement → defect |
---|
Setting this back to defect, since technically it's a regression, and regression fixes are *never* enhancements...
comment:5 Changed 9 years ago by
We should probably add an onerror listener for the script. The onerror event is supported in all modern browsers, and we could give a nice explicit error message in the environment that most devs work on. AFAICT, this is only unsupported in IE8 and earlier. For devs that actually dev on IE8 and earlier (poor souls), I don't really have any opinion on that.
comment:6 follow-up: 8 Changed 9 years ago by
Priority: | low → blocker |
---|
Restoring blocker status. I do not want 1.7.2 to go out without a resolution one way or another on this ticket. Error reporting is very important.
comment:7 Changed 9 years ago by
Kris' onerror handler sounds like a good idea.
FYI, I tried the latest code, running in firebug (randomly tested on page dojo/tests/parser/parser.html):
>>> dojo.require("foo.bar") undefined "NetworkError: 404 Not Found - http://localhost/trunk/foo/bar.js"
So AFAICT there's no regression. The new require() call though does fail silently:
>>> require(["foo/bar"], function(){ console.log("hi"); }); function()
comment:9 Changed 9 years ago by
In [27765] I implemented Kris's idea which seems reasonable. I also set the default timeout to 15s as per v1.6. I'm still not convinced any of this is anything more than code noise since these errors are easily visible in the network panel.
I'll let it sit for a while and see the reaction; anybody with a strong opinion should speak up...I'm not committed to keeping these changes yet.
comment:10 Changed 9 years ago by
Why did you add a var xx= 0;
? Obviously, I think that this change is great and will help make people feel less confused about what is going on with the new loader when it encounters an error. :)
comment:11 Changed 9 years ago by
Blocked By: | 14750 added |
---|
comment:6 follow-up: 8 Changed 9 years ago by
Just wondering: would it make any sense to backport the onerror stuff as well since 1.6 and earlier running in sync mode used to report an error to the console immediately upon load failure?
comment:7 Changed 9 years ago by
Not backporting the new machinery to catch script errors; that feels like too much risk for questionable reward.
Note that IE8 (probably 6 and 7) reports a loaded readystate change for a 404 error on a script element, so it is hard to detect when a script failed to load on IE. Since not all scripts/dependencies define modules, it's not as simple as just checking that all modules have arrived, and I don't want to start adding cruft to the loader for a very small user set. Therefore, IE8 and less users, you may not get a signal that a module failed to load.
In order to help this situation, I added the trace flag loader-define-nonmodule
, which will console.log everytime a nonmodule is detected...as will be the case when a 404 occurs on IE when trying to load a module. Here is an example that shows how to use it:
<html> <head> <script data-dojo-config="async:1, trace:{'loader-define-nonmodule':1}" src="dojo/dojo.js"> </script> <script> require(["some/nonexisting/module"], function(value){ console.log(value); }); </script> </head> </html>
Upon attempting to load this in IE==8, you will see the console message:
LOG: trace:loader-define-nonmodule:some/nonexisting/module
comment:8 Changed 9 years ago by
Replying to csnover:
Just wondering: would it make any sense to backport the onerror stuff as well since 1.6 and earlier running in sync mode used to report an error to the console immediately upon load failure?
You will get an XHR error in sync mode that is clearly visible in the console in IE. Other browsers will timout (in addition to the console error).
Not outputting a warning msg to the console consequent to a programmer's error is not a blocker.
The timeout is configurable; if you want 15 seconds, then do something like this:
I don't see how 15s is useful at all. If you're debugging a program, you're not going to wait around 15s for the program to do nothing. Why not go over to the network panel in the debugger and look for red?
My instinct is that the current code is reasonable. But I'll keep this open to let others voice opinions. If there is a strong desire to set a default wait time to something other than 0, then I'm happy to make that change.