datadog-static-analyzer icon indicating copy to clipboard operation
datadog-static-analyzer copied to clipboard

Add option to specify heap memory limit to underlying JavaScript runtime

Open pranavpudasaini opened this issue 1 year ago • 8 comments

Is your feature request related to a problem? Please describe. While scanning a repo with 4000+ TypeScript files with a total of 100+ rules, it always crashes with the error: "Fatal JavaScript out of memory: Reached heap limit". I'm using a standard GitHub-hosted runner for private repos with 2 cores and 7 GiB memory to perform the test.

Describe the solution you'd like It would be awesome if there were an option to specify the maximum heap size for the underlying JavaScript runtime. Could be something equivalent to NODE_OPTIONS=--max_old_space_size=2048.

Describe alternatives you've considered I've tried specifying heap size by adding the command prefix NODE_OPTIONS=--max_old_space_size=2048 to the datadog-static-analyzer binary but the JS runtime doesn't seem to use it.

image

pranavpudasaini avatar May 16 '24 17:05 pranavpudasaini

Thanks for the report.

We have been aware of this issue, and we are currently working on some changes that will (among other things) prevent this particular crash.

While we do not currently expose configuration options for memory usage, we may in the future--similarly to how we allow max CPU core configuration.

--

In the meantime, we have adjusted our default rulesets to temporarily disable some static analysis rules that exacerbate this issue. Hopefully that bandaid solves your particular issue, but if not, I'll update this issue when the fixes I mentioned get merged and released.

jasonforal avatar May 16 '24 19:05 jasonforal

@jasonforal Thank you for your response!

I've tried running the scans against the same repo but I'm still getting the same issue. We will switch to a larger runner instance until the heap configuration is available.

pranavpudasaini avatar May 17 '24 04:05 pranavpudasaini

@pranavpudasaini thank you for the report. Could you please run the analyzer with the option --debug yes (if you use the GitHub action, you can specify it as an input).

This will gives the output about which rule is failing and will help us finding the culprit.

juli1 avatar May 17 '24 11:05 juli1

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

typescript-code-style/ban-tslint-comment
typescript-code-style/ban-ts-comment
typescript-code-style/no-confusing-non-null-assertion
typescript-code-style/no-empty-interface
typescript-code-style/no-inferrable-types
typescript-code-style/no-useless-empty-export
typescript-code-style/class-name
typescript-code-style/function-naming
typescript-code-style/parameter-name
typescript-code-style/method-name
typescript-code-style/no-duplicate-imports

pranavpudasaini avatar May 17 '24 15:05 pranavpudasaini

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

Any change you can share the file that causes the crash?

juli1 avatar May 22 '24 12:05 juli1

same issue is also happening to us and it's very frustrating

SophisticaSean avatar Jun 07 '24 20:06 SophisticaSean

We are working on removing all operations that trigger these issues. We should be done by end of month.

juli1 avatar Jun 07 '24 21:06 juli1

Hello @pranavpudasaini and @SophisticaSean, we released a new version of the analyzer (v0.3.7) that should circumvent the initial issue reported (this update greatly reduces v8 allocations, so hitting the heap limit should "never" happen under normal usage).

However, I will be keeping this issue open because v8 will still crash if it hits the heap limit -- we will eventually be adding functionality to gracefully handle this and treat it much like we do an execution timeout.

jasonforal avatar Jul 17 '24 14:07 jasonforal