Opened 7 years ago

Closed 7 years ago

#15441 closed defect (fixed)

Parser swallows errors

Reported by: Mike Wilcox Owned by: Kitson Kelly
Priority: blocker Milestone: 1.8
Component: Parser Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

A recent commit that allows the parser to asynchronously load AMD packages using promises, which I've tracked down as the culprit to swallowing errors.

For a test, simply add something like: foo=bar; (undefined global var) in a postCreate in a widget that is parsed in markup.

I was under the impression that this async parse feature would be an opt-in?

Refs: #15118 Commit: [28464/dojo]

Attachments (1)

parser.diff (656 bytes) - added by Mike Wilcox 7 years ago.
parser error catcher

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by Mike Wilcox

Cc: Kitson Kelly added
Milestone: tbd1.7.3
Priority: undecidedhigh

comment:2 Changed 7 years ago by Colin Snover

Milestone: 1.7.31.8

Do not understand why milestone was set to 1.7.3 since this is a feature that only exists in 1.8.

comment:3 Changed 7 years ago by bill

Cc: Kitson Kelly removed
Owner: changed from bill to Kitson Kelly
Status: newassigned

Yah, that milestone didn't make sense to me either.

The parser features are not opt-in, and as you said, the asynchronous support makes error reporting more difficult, specifically that it needs to be done through the Deferred, calling reject(). However, in the common case when the parser runs synchronously it would be good to throw the errors synchronously, like in the old days. Probably at the end of parse() etc. we should throw if the promise is fulfilled and has an error condition.

I think Kitson said he was going to work on this.

Changed 7 years ago by Mike Wilcox

Attachment: parser.diff added

parser error catcher

comment:4 Changed 7 years ago by Mike Wilcox

Kitson, I attached a patch that I think is all that should be needed. I'm not seeing an indication that Deferred will handle uncaught errors, so imho you should do it in the parser.

This will throw two errors if you attach your own handler like parser.parse().then(handlers...). But I believe that side effect is harmless.

comment:5 Changed 7 years ago by Kitson Kelly

Priority: highblocker

comment:6 Changed 7 years ago by Kitson Kelly

Resolution: fixed
Status: assignedclosed

In [28851]:

Changes to dojo/parser does not swallow errors, inspired by mwilcox, fixes #15441 !strict

Note: See TracTickets for help on using tickets.