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

fix: catch RecursionError in initialization #446

Merged

Conversation

tomoto
Copy link
Contributor

@tomoto tomoto commented Dec 8, 2024

This is the suggested fix for #445. Please see #445 for the exact description about what this fix is trying to address.

The issue can be addressed by catching the errors raised from result.get() (receivning the result from the child process) in LangServer.workspace_init. The caught error can be convereted to string so that it will be posted as an error message.

With this fix, the fortls reports an "Initialization failed" message for the file that caused the RecusrionError, and successfully completes the initialization instead of crashing itself. It looks like below in VSCode (The red underline indicates the file that caused the issue).

image

@tomoto tomoto requested a review from gnikit as a code owner December 8, 2024 14:31
@gnikit gnikit force-pushed the fix/recursion-error-in-initialization branch from 3c17071 to 87dc8f4 Compare February 28, 2025 13:59
@gnikit
Copy link
Member

gnikit commented Feb 28, 2025

I'll have a look at this a bit later today (hopefully). I'll play with it a bit and see if there is a nice way of reporting this error + testing

Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.33%. Comparing base (9a86506) to head (f00b8ea).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   88.26%   88.33%   +0.06%     
==========================================
  Files          35       35              
  Lines        4797     4800       +3     
==========================================
+ Hits         4234     4240       +6     
+ Misses        563      560       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gnikit gnikit force-pushed the fix/recursion-error-in-initialization branch from 87dc8f4 to def63f4 Compare March 1, 2025 11:41
gnikit added 2 commits March 1, 2025 22:33
multiprocessing.Pool does not retain the base exception
that caused it to raise.
As a result it is not possible under this setup
to explicitly capture the RecursionError that is
getting raised when Pool attempts to pickle
the AST from the parser.
@gnikit gnikit force-pushed the fix/recursion-error-in-initialization branch from 91ce467 to f00b8ea Compare March 1, 2025 22:58
@gnikit gnikit merged commit f7c3ecd into fortran-lang:master Mar 1, 2025
17 checks passed
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.

fortls crashes during initialization due to RecursionError
2 participants