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.

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

window.showQuickPick is not closed when promise is rejected #693

Closed
SamVerschueren opened this issue Nov 26, 2015 · 8 comments
Closed

window.showQuickPick is not closed when promise is rejected #693

SamVerschueren opened this issue Nov 26, 2015 · 8 comments
Assignees

Comments

@SamVerschueren
Copy link
Contributor

When showQuickPick is used with a promise, it shows a nice progress bar which is cool. But when the promise is rejected, it should close the quick pick in my opinion, which is not the case at the moment, the progress bar just keeps spinning.

return window.showQuickPick(new Promise((resolve, reject) => {
    setTimeout(() => {
        reject();
    }, 2000);
}));

// @bpasero

@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Nov 26, 2015
@bpasero bpasero added this to the Dec 2015 milestone Nov 26, 2015
@bpasero
Copy link
Member

bpasero commented Nov 26, 2015

I fixed most of the issues: If there is an error in your promise, the picker will hide and you have a chance to handle the error from the returned promise from the pick() call.

@jrieken is our proxy from main to PH not handling errors? I have the following extension that shows a pick with error but in the console I still get an error about unhandled promise:

export function activate(context: vscode.ExtensionContext) {

    var disposable = vscode.commands.registerCommand('extension.sayHello', () => {
        vscode.window.showQuickPick(new Promise((c, e) => {
            setTimeout(() => {
                e(new Error("does not work"));
            }, 2000);
        })).then(null, (e) => {
            vscode.window.showErrorMessage(e.toString());
        });
    });

    context.subscriptions.push(disposable);
}

image

@SamVerschueren
Copy link
Contributor Author

Thanks for the quick fix!

@jrieken
Copy link
Member

jrieken commented Nov 26, 2015

@bpasero The reject works as expected but there is an known issue with unhandled promise rejection. Our handler sits here: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/node/pluginHostProcess.ts#L33 but there is a well known issue with that. Since project rejection and handling can happen async (on different event loop ticks) a promise might be handled some time in the future. Since the node events seem to lack the actual promise causing this we can't have better logging.

@jrieken jrieken closed this as completed Nov 26, 2015
@jrieken jrieken added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Nov 26, 2015
@bpasero
Copy link
Member

bpasero commented Nov 26, 2015

@jrieken in the example code I gave, we have a valid use of our API and the console ends up to report unknown errors:

image

I would think this can happen from any promise that carries an error from the main side to the PH side. If we know this can happen and cannot fix it, we should filter these messages.

@bpasero bpasero reopened this Nov 26, 2015
@jrieken jrieken removed the bug Issue identified by VS Code Team member as probable bug label Nov 27, 2015
@jrieken jrieken removed this from the Dec 2015 milestone Nov 27, 2015
@jrieken jrieken removed their assignment Nov 27, 2015
@jrieken
Copy link
Member

jrieken commented Nov 27, 2015

It's not possible with the version of nodejs we are using today. Please read the comment and the links I have pointed to.

@jrieken
Copy link
Member

jrieken commented Nov 27, 2015

And since this only happens when the event loop is released between rejection and handling of it I think its the better compromise to see some potential false positive here and there then not seeing any unhandled rejection.

jrieken added a commit that referenced this issue Nov 27, 2015
@jrieken jrieken self-assigned this Nov 27, 2015
@jrieken jrieken removed the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Dec 3, 2015
@jrieken
Copy link
Member

jrieken commented Dec 3, 2015

tracking the false positive in unhandled rejection here #963

@jrieken jrieken closed this as completed Dec 3, 2015
@SamVerschueren
Copy link
Contributor Author

👍

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants