Skip to content

Conversation

mediremi
Copy link
Member

@mediremi mediremi commented Jul 28, 2025

Sometimes when attempting saving a ReScript file in VS Code, the formatter will appear to hang and this prevents the file from being saved:

By running ps aux | grep rescript, I discovered that the source of the problem was not the formatter but rescript-editor-analysis.exe:

$ ps aux | grep rescript
mediremi  219605 98.6  0.0 275740 12696 ?        R    20:33   0:14 /home/mediremi/projects/my_project/node_modules/@rescript/linux-x64/bin/rescript-editor-analysis.exe codeAction /home/mediremi/projects/my_project/SomeFile.res 99 9 99 9 /tmp/rescript_format_file_219371_9.res

Using the file from my codebase that had triggered this issue as a starting point, I was able to create the following ReScript file and bash script to reproduce the issue:

module Sqlite = {
  module Transaction = {
    type t<'a>

    external runImmediate: t<'a> => 'a = "immediate"
  }

  module Database = {
    type t

    external make: () => t = "default"

    external transaction: (t, unit => 'a) => Transaction.t<'a> = "transaction"
  }
}

let openSqliteDB = () => Sqlite.Database.make()

let transactionForProject = (fn) => {
  openSqliteDB()
  ->Sqlite.Database.transaction(fn)
  ->Sqlite.Transaction.runImmediate
  ->ignore
}
_build/default/analysis/bin/main.exe codeAction ./SomeFile.res 22 9 22 9 ./SomeFile.res

By using print_endline, I was able to pinpoint the cause of the hang to an infinite recursion in TypeUtils.resolveTypeForPipeCompletion. When TypeUtils.findReturnTypeOfFunctionAtLoc returns a type variable, resolveTypeForPipeCompletion will loop forever.

I then solved the issue by adding a depth arg to TypeUtils.resolveTypeForPipeCompletion that keeps track of the current level of recursion and makes that helper bail out after depth > 10

To prevent this issue, we just need to handle TypeUtils.findReturnTypeOfFunctionAtLoc returning a type variable by returning it rather than looping over it.

Copy link

pkg-pr-new bot commented Jul 28, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7731

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7731

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7731

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7731

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7731

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7731

commit: 702a414

@mediremi mediremi marked this pull request as ready for review July 29, 2025 08:28
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Thanks a lot! 🎉

@nojaf
Copy link
Member

nojaf commented Jul 29, 2025

Could you please provide more details here? This fix seems to be just a temporary solution, and it doesn’t address the underlying issue. What is causing the repeated recursion? It would be helpful if you could add a clear comment for anyone who might encounter this later. At the very least, please explain what you're working around. I don’t want to hold things up, but we shouldn’t merge this without more context about the structural implications. Thank you!

mediremi added 2 commits July 30, 2025 11:55
…orces a maximum iteration count and add TODO about fixing the root of the issue
@mediremi
Copy link
Member Author

@nojaf you're right, this only fixes the symptom and not the root issue, and I should have left a TODO regarding that.

I've now added a comment to the code about this 👍

match typFromLoc with
| Some typFromLoc ->
typFromLoc |> resolveTypeForPipeCompletion ~lhsLoc ~env ~package ~full
(* Prevent infinite loops when `typFromLoc` is a type variable by bailing out after
Copy link
Member

Choose a reason for hiding this comment

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

What exactly is typFromLoc in this case?
That is still not clear to me, which part of the example is passed here over and over?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure - I don't understand this code very well, I only tracked down this issue with a combination of adding log statements and binary search.

@mediremi mediremi force-pushed the fix-code-action-hang branch from bb1c492 to 44c4030 Compare July 30, 2025 12:13
@mediremi mediremi changed the title Limit TypeUtils.resolveTypeForPipeCompletion to 10 recursive calls to prevent infinite loops Prevent infinite recursion in TypeUtils.resolveTypeForPipeCompletion by not looping over type variables Jul 30, 2025
@zth zth enabled auto-merge (squash) July 30, 2025 12:22
@zth
Copy link
Member

zth commented Jul 30, 2025

@mediremi would you backport this to the VSCode repo as well? This bug should exist with v11 too.

@mediremi
Copy link
Member Author

I'll backport it now 👍

@zth zth merged commit 8057ded into rescript-lang:master Jul 30, 2025
27 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.

4 participants