Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 8, 2025

The kernel picker was not properly handling CancellationError exceptions thrown by third-party extensions' JupyterServerCommandProvider.handleCommand() implementations, causing the UI to remain stuck in a loading state instead of dismissing as documented in the API.

Problem

According to the Extension API documentation, when third-party extensions want to dismiss the kernel picker UI (e.g., when users hit ESC or close their custom dialogs), they should throw a CancellationError from their handleCommand method. However, this wasn't working correctly - the picker would stay in a pending/loading state instead of dismissing.

The API docs clearly state:

If however users exit the UI or workflow (if any provided by 3rd party extension) by selecting a close button or hitting the ESC key or the like, extensions are then expected to throw a CancellationError, else the previous UI will be once again, which might not be desirable.

Root Cause

The issue was in the caller's error handling in RemoteNotebookKernelSourceSelector. When any exception (including CancellationError) was thrown from selectRemoteServerFromRemoteKernelFinder(), the lazyQuickPick.busy = false statement was only executed on success, leaving the UI in a perpetual loading state when errors occurred.

Solution

Moved the lazyQuickPick.busy = false statement from the success path to the finally block:

try {
    if (lazyQuickPick) {
        lazyQuickPick.busy = true;
    }
    const ret = await this.selectRemoteServerFromRemoteKernelFinder(selectedSource, state, token);
    return ret;
} catch (ex) {
    // ... existing error handling
} finally {
    if (lazyQuickPick) {
        lazyQuickPick.busy = false;  // Always reset UI state
    }
    taskNb.dispose();
}

This ensures that:

  • UI loading state is properly dismissed when third-party extensions throw CancellationError
  • All other exceptions continue to work as expected
  • The change is minimal and surgical, affecting only the UI state management
  • No changes to error propagation logic - CancellationError still bubbles up correctly to dismiss the picker

Testing

  • Added unit test verifying the UI busy state management pattern
  • Verified all existing kernel-related tests still pass (no regressions)
  • Confirmed the fix resolves the UI stuck in loading state issue

Fixes #16469.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Extension API: Throwing a CancellationError does not dismiss the kernel picker. Fix CancellationError handling in kernel picker to properly dismiss UI Sep 8, 2025
Copilot AI requested a review from DonJayamanne September 8, 2025 01:19
Copilot finished work on behalf of DonJayamanne September 8, 2025 01:19
Copy link
Contributor

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I don't think the changes are valid, please review my comments and your changes

Copy link
Contributor

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address my comments

Copy link
Contributor Author

Copilot AI commented Sep 8, 2025

@copilot I don't think the changes are valid, please review my comments and your changes

I've reviewed and addressed your feedback. The original try-catch approach was indeed invalid because both branches performed identical actions.

The correct fix was to ensure the UI busy state gets reset in the finally block, not to change error handling logic. This ensures that when third-party extensions throw CancellationError to dismiss the picker, the UI actually dismisses instead of staying in a loading state.

Fixed in commit d99716c with the proper approach.

Copilot AI changed the title Fix CancellationError handling in kernel picker to properly dismiss UI Fix kernel picker UI stuck in loading state when third-party extensions throw CancellationError Sep 8, 2025
Copilot AI requested a review from DonJayamanne September 8, 2025 01:59
Copilot finished work on behalf of DonJayamanne September 8, 2025 01:59
Copy link
Contributor Author

Copilot AI commented Sep 8, 2025

@copilot address my comments

You're absolutely right about the redundant try-catch block in commit 79fbf67. I've already addressed this issue in commit d99716c by:

  1. Removing the redundant try-catch block that unnecessarily wrapped handleCommand (both branches did the same thing)
  2. Implementing the correct fix by moving lazyQuickPick.busy = false to the finally block

This ensures the UI busy state is always reset when CancellationError is thrown by third-party extensions, while keeping the code clean and purposeful.

Copilot finished work on behalf of DonJayamanne September 8, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extension API: Throwing a CancellationError does not dismiss the kernel picker.

2 participants