The debugger does not have an easy way to know when a source-map is not providing useful mappings
Categories
(DevTools :: Debugger, defect, P3)
Tracking
(Not tracked)
People
(Reporter: julienw, Unassigned)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [debugger-reserve])
Attachments
(2 files, 1 obsolete file)
image.png
STR:
1. Open the attached file.
2. Open the debugger.
3. Open script.js
4. Try to add breakpoints.
Expected:
* Breakpoints work.
Actual:
* Breakpoints don't work.
Additional notes:
1. I generated this code with webpack:
webpack --optimize-minimize --devtool inline-source-map script.js script-bundle.js
from the following code:
function test(number) {
const variable = number;
console.log(variable);
}
document.querySelector('button').addEventListener('click', function() {
test(5);
});
2. This works in Chrome. (tm)
3. This works fine when the script is in a separate file.
|
||
Updated•7 years ago
|
|
||
Updated•6 years ago
|
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
Reporter | |
Comment 2•5 years ago
|
||
Nope, this still doesn't work.
Note this is also extremely easy for you to test by yourself from the attached testcase :) There's a CSP error that, I think, doesn't change anything for this bug.
|
|
||
Comment 3•5 years ago
|
||
Thanks for testing. Your STR is great.
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
||
Comment 4•5 years ago
|
||
Honza, would you mind re-testing and seeing if this still occurs?
|
|
||
Comment 5•5 years ago
|
||
Okay, just tested and did not see breakpoints.
Brian, i'm curious what you think. We should be able to get the generated breakpoint locations
|
||
Comment 6•5 years ago
|
||
Still doesn't work.
BP can't be added
BP gutter is grey.
Honza
|
|
||
Updated•5 years ago
|
|
||
Comment 7•5 years ago
|
||
The problem here is related to a newline in the eval'ed script. This is the .html contents:
<!doctype html>
<html>
<head>
<meta charset='utf-8'>
<meta name='viewport' content='initial-scale=1'>
</head>
<body>
<button type="button">Display something!</button>
<script>
const script1 = `
!function(n){function t(o){if(e[o])return e[o].exports;var r=e[o]={i:o,l:!1,exports:{}};return n[o].call(r.exports,r,r.exports,t),r.l=!0,r.exports}var e={};t.m=n,t.c=e,t.d=function(n,e,o){t.o(n,e)||Object.defineProperty(n,e,{configurable:!1,enumerable:!0,get:o})},t.n=function(n){var e=n&&n.__esModule?function(){return n.default}:function(){return n};return t.d(e,"a",e),e},t.o=function(n,t){return Object.prototype.hasOwnProperty.call(n,t)},t.p="",t(t.s=0)}([function(n,t){function e(n){const t=n;console.log(t)}document.querySelector("button").addEventListener("click",function(){e(5)})}]);
//# sourceMappingURL=OMITTED FOR BREVITY
`;
eval(script1);
</script>
</body>
</html>
The generated code in the string assigned to script1 starts on line 2. I don't know much about source maps but from what I can tell the map describes the region of the generated code which maps back to the original scripts. In this case the region is all on line 1, starting at column 475. Because there are no scripts on line 1, we aren't able to find any positions to set breakpoints at. If I remove the newline after the ` character when assigning to script1, I can set and hit breakpoints in script.js as expected.
I guess we could deal with this by doing a fuzzy match that tolerates extra newlines in the generated source. Because of the mismatch between the generated code and the source map information, I think something like this would be required in order to handle breakpoints here. There's a limit to that sort of approach though; it will always be possible for there to be mismatches which we aren't able to recover from. An alternative would be to have some way to surface this in the UI and indicate to users that there seems to be a mismatch and breakpoints can't be set.
|
|
||
Updated•5 years ago
|
|
|
||
Comment 8•5 years ago
|
||
I agree that adding fuzzy matching is dangerous. Also, note that chrome does not handle this. So there are no parity concerns.
I like the idea of surfacing a warning to users. What kind of rules do you think make sense.
CCing jryans who might also have some ideas here too.
|
|
||
Comment 9•5 years ago
|
||
What do you think of considering this bug fixed and if we want to add better user warnings, creating a follow up bug?
|
|
||
Comment 10•5 years ago
|
||
(In reply to Jason Laster [:jlast] from comment #8)
I like the idea of surfacing a warning to users. What kind of rules do you think make sense.
In this case we could notice that no breakpoint positions were found for scripts in the range described by the source map, but it seems like there could be other cases of malformed input that wouldn't fit this criterion. It's hard to say what is possible / appropriate without more knowledge of what information is contained in source maps and how it is organized.
I agree it would be best to warn when things like this fail, since it strongly suggests a mismatch between the source map and the running script (could be a toolchain bug, etc).
In my view as a user of a debugger, it's very frustrating when breakpoints fail silently (either by auto-deleting the breakpoint I tried to set, or leaving it apparently set in the UI, but not actually stopping during execution).
For cases like this where we can clearly detect a mismatch, a warning seems great. Eventually, we could add more sophisticated checking of source map to see if it appears to be offset by N lines or characters, which strongly implies it contains nonsense that will likely do more harm than good.
|
|
Reporter | |
Comment 12•5 years ago
|
||
I was under the impression that Chrome works with this setup, but I just tried and it doesn't. It allows to set breakpoints but they don't work.
|
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
||
Comment 13•4 years ago
|
||
I'm on the fence here and tempted to call this WONTFIX since it feels like, while we can address this one very specific case of off-by-one causing no breakpoints, in the majority of cases I'd expect a map to seem mostly right. I'd be very concerned that any detection heuristic we add is either too specific to be useful or too broadly at risk of false-positives.
|
||
Comment 14•4 years ago
|
||
Logan, I am not unsure where "detection heuristic" apply – fixing breakpoints or detecting incorrect maps. Can you clarify if we can detect when mappings are incorrect?
|
|
||
Comment 15•4 years ago
|
||
I am not unsure where "detection heuristic" apply – fixing breakpoints or detecting incorrect maps.
I do not think attempting to fix broken maps is something we should do, so I'm focusing on detecting incorrect maps. In order to show a warning to users, we'll need to detect that the map is one that we consider broken. In the specific case of this example, we could write a validation step to say "when the user opens a file, do all of the mappings for the original file being viewed point at a non-empty line in the generated file", but I imagine that 99% of the time a map will pass that test even if it has issues because most broken maps aren't going to be broken in this particular way. So the question is, what is the check that we'd want to run in order to show a warning?
Another downside is that we'll need to fetch the generated source text and then search it based on the mapping locations. Normally the debugger doesn't try to fetch the generated text unless the user opens the editor since the text could be very large, and as mentioned above we don't normally parse all the mappings until they are needed.
There is also the question of when we run this check. Loading the generated source could be slow and parsing sourcemap mappings is expensive, so right now it is done lazily the first time something queries the mappings. If we want to validate the map on every map load, that's moving a lot of logic to be up-front computed, and then we'll have to do the actual validation which will also take time.
Can you clarify if we can detect when mappings are incorrect?
In my opinion, I don't think this is something we should try to do as an automatic feature.
I could see a motivation for wanting a tool to debug a page's sourcemaps in the devtools, but I question whether it would be beneficial enough for anyone to bother to use. There are already sourcemap-viewers on the internet and they'll only ever be used by people working on JS tooling since the average user has no way to address source-map issues even if they think their map is broken. If the end result either way, for the average user, is filing a bug in the repo of their build tooling, it's hard for me to see much for us to gain by building out proper tooling for it in FF itself.
|
|
||
Updated•4 years ago
|
|
|
||
Comment 16•4 years ago
•
|
||
(edit: removed incorrect comment)
Would it sound reasonable then to lazily warn when source maps fails to meet expectations (when we discover incorrect mappings, etc)? I think just having the warning with some link to learn more would already help.
|
|
||
Updated•4 years ago
|
|
|
||
Comment 17•4 years ago
|
||
From our discussion:
Do we have a point in time when we can say the source map is faulty? (e.g. variable mapping, scopes panel, etc). So, we can show the warning async. If this would de instrumented we could also have nice telemetry about how often source maps are broken...
Still might be a problem to identify that the source map is incorrect, but at least it would happen async.
Honza
|
|
||
Comment 18•4 years ago
|
||
|
||
Updated•4 years ago
|
|
|
||
Comment 19•4 years ago
|
||
Comment on attachment 9152220 [details]
Bug 1400859 - Pause in the original location by default. r=davidwalsh
Revision D77158 was moved to bug 1641731. Setting attachment 9152220 [details] to obsolete.
|
|
||
Updated•4 years ago
|
|
|
||
Updated•2 years ago
|
Description
•