Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Search: promise did not implement onCancel #44740

Closed
bpasero opened this issue Feb 28, 2018 · 7 comments
Closed

Search: promise did not implement onCancel #44740

bpasero opened this issue Feb 28, 2018 · 7 comments
Assignees
@roblourens @chrmarti
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Milestone
February 2018

Comments

@bpasero
Copy link
Member

bpasero commented Feb 28, 2018

I am not sure yet how to reproduce, but I see this error popping up in insiders console:

Error: Promise did not implement oncancel
    at n.Class.derive._oncancel._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:151:963)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._oncancel._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._oncancel.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:643)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._creator._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:332)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:153:351
    at Array.forEach (native)
    at n.Class.derive._oncancel.n.Class.derive.join [as _oncancel] (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:153:283)
    at n.Class.derive._oncancel._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:152:7)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._oncancel._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._oncancel.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._creator._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:332)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._creator._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:332)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._oncancel.e.search [as _oncancel] (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3956:96)
    at n.Class.derive._oncancel._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:152:7)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._oncancel._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._oncancel.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._creator._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:332)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:153:351
    at Array.forEach (native)
    at n.Class.derive._oncancel.n.Class.derive.join [as _oncancel] (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:153:283)
    at n.Class.derive._oncancel._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:152:7)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._oncancel._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._oncancel.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at n.Class.derive._creator._cancelAction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:332)
    at Object.enter (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:147:812)
    at n.Class.derive._creator._run (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:150:84)
    at n.Class.derive._creator.cancel (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:149:247)
    at c.cancelPendingSearch (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3835:232)
    at c.getResults (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3832:772)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3532:597
    at n.Class.define.cancel.then (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:151:517)
    at t.handleDefaultHandler (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3532:187)
    at t.onType (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3531:594)
    at Object.onType (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3527:614)
    at e.onType (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:967:780)
    at HTMLInputElement.<anonymous> (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:963:517)

It looks like it originates from the file quick open trying to cancel an active search.

I wonder why this suddenly shows up so much more often, did something change here @roblourens ?
@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Feb 28, 2018
@bpasero
Copy link
Member Author

bpasero commented Feb 28, 2018 •
edited

/cc @chrmarti

@bpasero bpasero self-assigned this Feb 28, 2018
@bpasero bpasero added this to the February 2018 milestone Feb 28, 2018
@chrmarti
Copy link
Contributor

The async awaiter generated by TypeScript breaks the cancel chain (introduced here 70e020b). I'll add a fix.

@bpasero
Copy link
Member Author

bpasero commented Feb 28, 2018

@chrmarti wow thanks for finding this!

@bpasero bpasero assigned chrmarti and unassigned bpasero Feb 28, 2018
@chrmarti chrmarti added the search Search widget and operation issues label Feb 28, 2018
@bpasero
Copy link
Member Author

bpasero commented Mar 1, 2018

Marking as verified as I am not seeing this in todays insider build.

@bpasero bpasero added the verified Verification succeeded label Mar 1, 2018
@roblourens
Copy link
Member

Does this just mean we can't use async/await when we use cancel? Which means we have to be very careful anywhere it's used in core? Didn't realize that.

@bpasero
Copy link
Member Author

bpasero commented Mar 6, 2018

@roblourens I think this means that as much as possible we must stay away from using WinJS.Promise.cancel() and introduce a different concept instead (CancellationToken).

@chrmarti
Copy link
Contributor

chrmarti commented Mar 6, 2018

The TypeScript helper for async/await knows which promise to instantiate based on the declared return type on the async function, but it doesn't know about the cancel handler passed as an additional argument to the WinJS promise. When there is no return type declared, it just uses the native promise, which doesn't even have a cancel method. (See http://jsbin.com/mepixob/4/edit?html,js,output for my investigation.)

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@roblourens roblourens

@chrmarti chrmarti

Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Projects
None yet
Milestone
February 2018
Development

No branches or pull requests
3 participants
@roblourens @bpasero @chrmarti