Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#16547 closed defect (fixed)

[patch] [cla] CancelError emitted to error console

Reported by: Akira Sudoh Owned by: dylan
Priority: undecided Milestone: 1.13
Component: Core Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

In 1.7 version of dojo/_base/Deferred.js, there is code not to emit CancelError:

if(!error || error.log !== false){
    (dojo.config.deferredOnError || function(x){ console.error(x); })(error);
}

However, it's gone in 1.8, even in back-compat dojo/_base/Deferred.js, like:

if(has("config-useDeferredInstrumentation")){
    if(NewDeferred.instrumentRejected){
        NewDeferred.instrumentRejected(error, !!nextListener);
    }
}

Cannot see the code not to emit CancelError in instrumentation code, either.

Attached a test case and a patch.

Attachments (2)

testDeferred.html (644 bytes) - added by Akira Sudoh 7 years ago.
Test case, /dojo-1.8 portion of this file should be replaced with the correct path of Dojo Toolkit (1.7 or 1.8)
instrumentation.js.diff (444 bytes) - added by Akira Sudoh 7 years ago.
A patch

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by Akira Sudoh

Attachment: testDeferred.html added

Test case, /dojo-1.8 portion of this file should be replaced with the correct path of Dojo Toolkit (1.7 or 1.8)

Changed 7 years ago by Akira Sudoh

Attachment: instrumentation.js.diff added

A patch

comment:1 Changed 7 years ago by bill

Component: GeneralCore
Owner: set to Mark Wubben
Status: newassigned
Summary: CancelError emitted to error console[patch] [cla] CancelError emitted to error console

comment:2 Changed 6 years ago by Mark Wubben

Seems sensible, though dojo/Deferred doesn't set .log = false on cancelation errors, so those would still be reported.

Error reporting in Dojo's promise implementation is underdeveloped, although there are some suggestions in <https://github.com/promises-aplus/unhandled-rejections-spec/issues?sort=updated&state=open> that would improve matters. Even with those changes though I'm not sure whether cancel errors should be suppressed.

Bill, thoughts?

comment:3 Changed 6 years ago by bill

Most (or all?) of the time, cancel "errors" aren't actually errors. For example, when FilteringSelect sends and XHR to the server but then the user types more characters before the XHR completes, it cancels the original XHR and sends a new one. Another case is if the user clicks to open a TitlePane or TreeNode, but then before the animation is complete, the user clicks again, canceling the original animation and starting a new one (to close rather than open).

Actually I don't see why you would ever want to report cancel errors to the console.

comment:4 Changed 6 years ago by Mark Wubben

Fair enough.

Best solution is probably to set log: false in dojo/errors/CancelError and remove that line from dojo/_base/Deferred. Then only pure cancel errors are ignored. The condition in dojo/promise/instrumentation should be

if(error && error.log === false){
  return;
}

I'm not going to figure out Subversion anymore though, so perhaps somebody else can pick this up.

comment:5 Changed 4 years ago by dylan

Milestone: tbd1.12

Cannot open ticket until #18154 is updated, so trying to batch modify a milestone for now.

comment:6 Changed 4 years ago by dylan

Milestone: 1.131.11
Owner: changed from Mark Wubben to dylan

comment:7 Changed 3 years ago by dylan

PR created at https://github.com/dojo/dojo/pull/201 . Need to add a test. Will look at this more tomorrow, but if anyone has feedback or suggestions, now's the time.

comment:8 Changed 3 years ago by dylan

Moving to 1.12 to give more time to consider this change.

comment:9 Changed 3 years ago by dylan

Milestone: 1.111.12

comment:10 Changed 3 years ago by dylan

Milestone: 1.121.13

Ticket planning... move current 1.12 tickets out to 1.13 that likely won't get fixed in 1.12.

comment:11 Changed 3 years ago by Michael Van Sickle <mvansickle@…>

Resolution: fixed
Status: assignedclosed

In 47d6dde/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:12 Changed 3 years ago by Michael Van Sickle <mvansickle@…>

In 899cf40/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.