Closed Bug 1027157 Opened 6 years ago Closed 6 years ago

Some warnings show up as "errors" in the console due to using JSEXN_TYPEERROR or JSEXN_SYNTAXERROR, etc

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox30 --- wontfix
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(2 files)

Attached image example of confusion
Despite each of these messages always being logged as a warning, they use error types rather than warning types. We should make them into proper warnings because right now they look like full on errors and it is confusing because they don't actually halt JS execution.

http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#263
http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#356
http://dxr.mozilla.org/mozilla-central/source/js/src/js.msg#400
Jeff, what's the best way to do this? Can we just create JSEXN_SYNTAXWARN, etc or something like that?
Flags: needinfo?(jwalden+bmo)
So I think just changing the error type to JSEXN_NONE is the short-term fix here.  Longer-term, we need to actually have a warning mechanism not conflated with the error/exception mechanism.
Flags: needinfo?(jwalden+bmo)
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
This just switches to JSEXN_NONE.
Attachment #8443096 - Flags: review?(jwalden+bmo)
Attachment #8443096 - Flags: review?(jwalden+bmo) → review+
Nick: do you think we should uplift this fix to Aurora 32 or even Beta 31? This is a small fix and the bug  may confuse to web developers, especially when they are using third-party JS libraries that cause these "errors".

These warnings were introduced in Firefox 30 (bug 948227), so fixing Beta 31 would soon correct this confusion about bogus TypeErrors.
Blocks: 948227
Flags: needinfo?(nfitzgerald)
(In reply to Chris Peterson (:cpeterson) from comment #6)
> Nick: do you think we should uplift this fix to Aurora 32 or even Beta 31?
> This is a small fix and the bug  may confuse to web developers, especially
> when they are using third-party JS libraries that cause these "errors".
> 
> These warnings were introduced in Firefox 30 (bug 948227), so fixing Beta 31
> would soon correct this confusion about bogus TypeErrors.

Yes I do.

I was going to wait for the patch to land on m-c before requesting approval. Pretty simple patch though, definitely shouldn't break anything.
Flags: needinfo?(nfitzgerald)
https://hg.mozilla.org/mozilla-central/rev/57d65c06db19
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8443096 [details] [diff] [review]
warnings-not-errors.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 948227, among others
[User impact if declined]: Warnings show up as errors in the console, which is lying to the user and can be confusing when you notice your JS execution doesn't actually halt and no exception gets thrown.
[Describe test coverage new/current, TBPL]: Has been on m-c for a while now, no issues.
[Risks and why]: Very little risk. Literally just changing JSEXN_SOMEKINDOFERROR to JSEXN_NONE so that it isn't reported as an error.
[String/UUID change made/needed]: None.
Attachment #8443096 - Flags: approval-mozilla-beta?
Attachment #8443096 - Flags: approval-mozilla-aurora?
Attachment #8443096 - Flags: approval-mozilla-beta?
Attachment #8443096 - Flags: approval-mozilla-beta+
Attachment #8443096 - Flags: approval-mozilla-aurora?
Attachment #8443096 - Flags: approval-mozilla-aurora+
checkin-needed is not needed. approval-mozilla-* is enough.
Keywords: checkin-needed
My bad! Thanks!
Duplicate of this bug: 926595
You need to log in before you can comment on or make changes to this bug.