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

Markup cell initialize fail on window reload #161520

Closed
rebornix opened this issue Sep 22, 2022 · 3 comments · Fixed by #162472
Closed

Markup cell initialize fail on window reload #161520

rebornix opened this issue Sep 22, 2022 · 3 comments · Fixed by #162472
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority insiders-released Patch has been released in VS Code Insiders notebook-markdown verified Verification succeeded

Comments

@rebornix
Copy link
Member

rebornix commented Sep 22, 2022

This is hard to have stable reproduce but almost every one in the notebook v-team ran into this in last few weeks: when reloading the window with a notebook editor open, the notebook editor might not open successfully, closing it and reopening the same document can fix it.

Debugged with log points in Insiders and I found it failed in BacklayerWebview#initializeMarkup

if (this._disposed) {
return;
}
// TODO: use proper handler
const p = new Promise<void>(resolve => {
const sub = this.webview?.onMessage(e => {
if (e.message.type === 'initializedMarkup') {
resolve();
sub?.dispose();
}
});
});
for (const cell of cells) {
this.markupPreviewMapping.set(cell.cellId, cell);
}
this._sendMessageToWebview({
type: 'initializeMarkup',
cells,
});
await p;

The initializeMarkup message is sent to the webview (at least we called the webview postMessage) but the response never came back. When this happens, the webview/iframe contains the html content, Jupyter notebook renderers' styles, but it doesn't have anything related to the markup renderers (no markup css).

@mjbvz please help take a look, we can have a call to do some debugging together.

@rebornix rebornix added bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority notebook-markdown labels Sep 22, 2022
@rebornix rebornix added this to the September 2022 milestone Sep 22, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue Sep 23, 2022
- initializeMarkup should always fire `initializedMarkup`, even if an exception is thrown while initializing

- `MarkupCell` should always resolve its `ready` property, even if an exception is thrown during rendering

Unclear if either of these were the causes of microsoft#161520, but still good to fix them
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 23, 2022

@rebornix I haven't seen this myself but addressed some possible causes with #161550. Let me know if you can reliably reproduce this issue

3f92eb6 also fixed some issues where if a renderer failed to load or threw during loading, we would never load the notebook

mjbvz added a commit that referenced this issue Sep 23, 2022
- initializeMarkup should always fire `initializedMarkup`, even if an exception is thrown while initializing

- `MarkupCell` should always resolve its `ready` property, even if an exception is thrown during rendering

Unclear if either of these were the causes of #161520, but still good to fix them
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 27, 2022

Closing since we haven’t seen this happening again recently

@mjbvz mjbvz closed this as completed Sep 27, 2022
@IanMatthewHuff
Copy link
Member

@rebornix @mjbvz Is it this issue here with a blue cylon but the main document never loads?
image

I still get this. I opened a document and reloaded and got this on the fourth try (current insiders). Anything I can collect when this happens to help out?

@IanMatthewHuff IanMatthewHuff added the verification-found Issue verification failed label Sep 29, 2022
@mjbvz mjbvz modified the milestones: September 2022, October 2022 Sep 30, 2022
@mjbvz mjbvz added the candidate Issue identified as probable candidate for fixing in the next release label Sep 30, 2022
@mjbvz mjbvz reopened this Sep 30, 2022
@mjbvz mjbvz modified the milestones: September 2022, October 2022 Sep 30, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue Sep 30, 2022
This tries to fix microsoft#161520 by doing the following:

- Wait for basic initialization to finish before posting messages
- `initializedMarkup` should wait  for the correct response instead of the first response

This fixes the issue in my testing, but causes the layout to shift around during load
@rebornix rebornix modified the milestones: October 2022, September 2022 Oct 4, 2022
mjbvz added a commit that referenced this issue Oct 4, 2022
* Fix race when loading notebook webviews

This tries to fix #161520 by doing the following:

- Wait for basic initialization to finish before posting messages
- `initializedMarkup` should wait  for the correct response instead of the first response

This fixes the issue in my testing, but causes the layout to shift around during load

* wait for js preload to be initialized in webview

* Removed pending message queue

* Remove extra async

Co-authored-by: rebornix <[email protected]>
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Oct 4, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue Oct 4, 2022
* Fix race when loading notebook webviews

This tries to fix microsoft#161520 by doing the following:

- Wait for basic initialization to finish before posting messages
- `initializedMarkup` should wait  for the correct response instead of the first response

This fixes the issue in my testing, but causes the layout to shift around during load

* wait for js preload to be initialized in webview

* Removed pending message queue

* Remove extra async

Co-authored-by: rebornix <[email protected]>
mjbvz added a commit that referenced this issue Oct 4, 2022
* Fix race when loading notebook webviews

This tries to fix #161520 by doing the following:

- Wait for basic initialization to finish before posting messages
- `initializedMarkup` should wait  for the correct response instead of the first response

This fixes the issue in my testing, but causes the layout to shift around during load

* wait for js preload to be initialized in webview

* Removed pending message queue

* Remove extra async

Co-authored-by: rebornix <[email protected]>

Co-authored-by: rebornix <[email protected]>
@rzhao271 rzhao271 added verified Verification succeeded and removed verification-found Issue verification failed labels Oct 5, 2022
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Oct 7, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority insiders-released Patch has been released in VS Code Insiders notebook-markdown verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants