Looping in firefox-dev as well, as I thin this is an important discussion.
On 18/10/2017 09:28, David Teller wrote:
Hi everyone,
Yesterday, Nightly was broken on Linux and MacOS because of a typo in
JS code [1]. If I understand correctly, this triggered the usual
"undefined is not a function", which was
1/ uncaught during testing, as these things often are;
Part of the reason it was uncaught, is that there's no automated test
coverage for that bit of code. It is a migration from one version to the
other, but unless there is an explicit test (and I don't see one) that
line won't be hit.
This seems to be a common theme with various migration routes in the
code base, when I've asked about ensuring more automated testing there
before I feel like I've got shrugs.
Admittedly, a lot of it is simple code, and should be correct, but as
we've seen, mistakes can be made. Should we now be changing our policy?
At a minimum, I wonder if we could write some sort of simple test for
CustomizableUI that would go through most routes (with minimal updates
for each change).
2/ basically impossible to diagnose in the wild, because there was no
error message of any kind.
I did an experiment, and the only way I got an error out was to have
"javascript.options.strict" on and "browser.dom.window.dump.enabled".
Unfortunately the breakage was such that the browser console couldn't be
opened, so you'd just having strict on wasn't enough.
I remember that we had bugs of this kind lurking for years in our
codebase, in code that was triggered daily and that everybody believed
to be tested.
I'd like to think that there is a better way to handle these bugs,
without waiting for them to explode in our user's face. Opening this
thread to see if we can find a way to somehow "solve" these bugs, either
by making them impossible, or by making them much easier to solve.
ESLint has caught some bugs - mainly undefined and unused related
issues, and is spread through most of the production javascript code.
Unfortunately it isn't able to catch this class of error. For that, we'd
need something like Flow. Last time I looked at it, it didn't feel like
a good fit for us, although I didn't go too deep, and I think there may
have been other people that were looking at it.
I have one proposal. Could we change the behavior of the JS VM as follows?
- The changes affect only Nightly.
- The changes affect only mozilla chrome code (including system add-ons
but not user add-ons or test code).
- Any programmer error (e.g. SyntaxError) causes a crash a crash that
displays (and attaches to the CrashReporter) both the JS stack in and
the native stack.
- Any SyntaxError is a programmer error.
- Any TypeError is a programmer error.
I expect that this will find a number of lurking errorsy, so we may want
to migrate code progressively, using a directive, say "use strict
moz-platform" and static analysis to help encourage using this directive.
It would definitely be interesting to fail tests on some of the strict
failures. I was surprised when I recently turned on the pref again to
see how many warnings there were.
Having looked through a few of those, I've just found at least one issue
<https://bugzilla.mozilla.org/show_bug.cgi?id=1409691>, though
non-critical, and I'm thinking we want to get the no-unused-expressions
<https://bugzilla.mozilla.org/show_bug.cgi?id=1409693> rule turned on
for ESLint as a result.
Mark
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
