Skip to content

Handle languageclient errors cases more robustly - #859

Merged
kaisalmen merged 4 commits into
mainfrom
discussion-855
Feb 14, 2025
Merged

Handle languageclient errors cases more robustly#859
kaisalmen merged 4 commits into
mainfrom
discussion-855

Conversation

@kaisalmen

@kaisalmen kaisalmen commented Feb 14, 2025

Copy link
Copy Markdown
Collaborator

I moved some tests around (wrong places) as well.
Fixing the wrapper handling in case of error introduced required changes to the react component and to several tests.

After thinking about #855

@kaisalmen
kaisalmen requested a review from CGNonofr as a code owner February 14, 2025 11:40
@kaisalmen

Copy link
Copy Markdown
Collaborator Author

@CGNonofr I finally understood why awaiting the language client on connection error broke the await in the test and it is now fixed with: 8c2b8f8

In a real application you should always define your own error handler!

@kaisalmen

Copy link
Copy Markdown
Collaborator Author

@CGNonofr last commit is for the problem @cdietrich discovered with the react component introduced by 6.3.0 version. We now have 78 unit tests.

@CGNonofr CGNonofr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not easy to review! 😢 LGTM though

@kaisalmen

kaisalmen commented Feb 14, 2025

Copy link
Copy Markdown
Collaborator Author

@CGNonofr sorry, once I started fixing the tests the domino effect started.

@kaisalmen
kaisalmen merged commit e93d21e into main Feb 14, 2025
@kaisalmen
kaisalmen deleted the discussion-855 branch February 14, 2025 15:44
@CGNonofr

CGNonofr commented Feb 14, 2025

Copy link
Copy Markdown
Collaborator

@CGNonofr sorry, once I started fixing the tests the domino effect started.

As always, same for me 😆

@kaisalmen

Copy link
Copy Markdown
Collaborator Author

@CGNonofr These changes made the Unknown Error: ResizeObserver loop completed with undelivered notifications. in the tests go away. The question now is why exactly, but if we find an answer this could be interesting. The monaco-vscode-api version was unchanged, btw.

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.

2 participants