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

Notification.promise --> [reject|resolve] nondeterministic behavior #15558

Open
gplasky opened this issue Dec 21, 2023 · 2 comments
Open

Notification.promise --> [reject|resolve] nondeterministic behavior #15558

gplasky opened this issue Dec 21, 2023 · 2 comments

Comments

@gplasky
Copy link

gplasky commented Dec 21, 2023

Description

I'm building a Jupyter frontend extension and I've created an in-progress notification for an async function similar to what's described in the notification example.

It works well but in my testing I've discovered what I believe is an edge case: The on-screen toast for the in-progress notification doesn't get updated despite the .reject() firing and the reject notification appearing in the notification center (instead of updating the on-screen toast). It appears to only happen once after the initial page load.

Reproduce

  1. Load Jupyterlab with my extension enabled and auto-activated.
  2. Trigger an in-progress notification with Notification.promise(PromiseDelegate, ...) that then instantly gets rejected with <PromiseDelegate>.reject(...) (e.g. in my case from an async function making an HTTP call which rejects its promise due to ERR_CONNECTION_REFUSED).
  3. Observe the in-progress toast remains on screen without displaying the expected error from Notification.reject() [1]
  4. Observe the toast in Step 3 continues to remain on-screen.
  5. Observe that the Notification.reject() notification is actually shown in the notification center. [2]

[1]
image

[2]
image

Expected behavior

  1. Load Jupyterlab with my extension enabled and auto-activated.
  2. Trigger an in-progress notification with Notification.promise(PromiseDelegate, ...) that then instantly gets rejected (e.g. in my case due to the async function rejecting its promise due to ERR_CONNECTION_REFUSED) with <PromiseDelegate>.reject(...).
  3. Observe the in-progress notification update to display an error. [3]

[3]
image

Context

While this only happens on the first in-progress notification after initial page load, there does not appear to be any correlation with how long after page load this happens. I've been able to reproduce it immediately after page load as well as a number of minutes afterward. After the first time it happens, successive (immediate) failures, as described above, successfully update the in-progress notification as expected.

Workaround

Introducing an artificial delay immediately before the PromiseDelegate.reject(...) call successfully prevents this issue (100ms seems to suffice). This leads me to believe there is something with the Notification system that is lazy loading or otherwise not immediately initializing.

Example

Initialize the promise

        this._fetchToastPromiseDelegate = new PromiseDelegate<string>;
        Notification.promise(this._fetchToastPromiseDelegate.promise, ...);

Add delay before rejection

        } catch (err) {
            await new Promise(f => setTimeout(f, 100));
            this._fetchToastPromiseDelegate.reject("Error fetching results. Consult the console logs for details.");
            httpErrorHandler(err);
@gplasky gplasky added the bug label Dec 21, 2023
Copy link

welcome bot commented Dec 21, 2023

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Dec 21, 2023
@JasonWeill JasonWeill added tag:Extensions and removed status:Needs Triage Applied to new issues that need triage labels Jan 2, 2024
@JasonWeill
Copy link
Contributor

@gplasky Thank you for opening this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants