forcedotcom / sfdx-scanner

MIT License
215 stars 49 forks source link

[BUG] DFA Scan timesout with a InternalExecutionError #1059

Open hen90 opened 1 year ago

hen90 commented 1 year ago

Describe the bug Running dfa command is timing out on one specific method. Have run with a rule-thread-timeout up to 14 hours and it will still timeout after this length of time, with this result in the output file:

Rule : InternalExecutionError
Description : Path evaluation timed out after 52000000 ms
URL: https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#ApexFlsViolationRule

We have looked at your OutOfMemory documentation and run the scan with the following switches

sfdx scanner:run:dfa --target "./*/.*" --projectdir "./" -f csv --rule-thread-timeout=52000000 --pathexplimit -1 --rule-thread-count 2 --sfgejvmargs "-Xmx4g" -o "C:\Users\xxxx\xxxx\scanresults20.csv"

but still have the issue

To Reproduce run scan: sfdx scanner:run:dfa --target "./*/.*" --projectdir "./" -f csv --rule-thread-timeout=52000000 --pathexplimit -1 --rule-thread-count 2 --sfgejvmargs "-Xmx4g" -o "C:\Users\xxxx\xxxx\scanresults20.csv"

Expected behavior I would expect this to run even for our complex codebase

Desktop (please complete the following information):

Additional context We fixed all other violations that were highlighted by the scan (these were all NullPointerExceptions) and these fixes were applied to the failing method so we don't believe there are further issues in that code, however we can't complete the scan. The failing method is complex with a Standard Cyclomatic Complexity of 28, but this is the nature of apex when working with complex objects in the metadata api and the complexity cannot be reduced.

"Workaround": No workaround found.

"Urgency": It will be business stopping if this causes us to fail our ISV security reviews.

rmohan20 commented 1 year ago

Hi @hen90 - thanks for reporting the issue. Appreciate that you took the time to read through the OutOfMemory documentation and experimented with tweaking the run parameters.

If the scanner:run:dfa command is failing on one specific method, seems worth trying to target exactly that method. While this still won't have all the results in one place, it can at least help find how the paths originating from this complex method are doing.

Also, if you can go more than 4g of heap space, that could help handle more complexity as well. Some complex projects that I test often need around 20g to reach completion.

Modifying the command you posted above, I'd recommend something like this:

sfdx scanner:run:dfa --target "./full/path/to/ApexClass.cls#yourMethod" --projectdir "./" -f csv --rule-thread-timeout=52000000 --pathexplimit -1 --rule-thread-count 2 --sfgejvmargs "-Xmx10g" -o "C:\Users\xxxx\xxxx\scanresults20.csv"

where:

Please let me know how this works for you.

rmohan20 commented 1 year ago

@hen90 Any luck? Were you able to get the remaining rows scanned?

hen90 commented 1 year ago

Hi,

sorry for the late reply. Still having timeouts even after your suggestions and even after running on a colleagues more powerful machine with 20g max heap and for 20 hours.

This Warning appears in the log literally hundreds of times, can you shine any light? What would EmptyReferenceExpression point to, is that the same as Null Reference? However this apiEnabledSessionId method is called pretty much by every method and doesn't time out in those. There are 4 sub methods in the main failing method and when running these separately they scanned without issue even when calling the apiEnabledSessionId method,

2023-05-08 11:03:40 dc6c7d0a-7321-4587-816c-bce188360bce WARN PathScopeVisitor:900 - TODO: Handle multiple levels. vertex=AssignmentExpression{properties={FirstChild=true, Operator==, BeginLine=788, DefiningType_CaseSafe=xxxx_configurationwizardcontroller, LastChild=true, DefiningType=xxxx_ConfigurationWizardController, EndLine=788, childIdx=0, BeginColumn=9}} , lhs=VariableExpression{properties={FirstChild=true, BeginLine=788, DefiningType_CaseSafe=xxxx_configurationwizardcontroller, LastChild=false, DefiningType=xxxx_ConfigurationWizardController, EndLine=788, Name_CaseSafe=sessionid, childIdx=0, BeginColumn=31, Name=sessionId}}

2023-05-08 11:03:40 dc6c7d0a-7321-4587-816c-bce188360bce WARN PathScopeVisitor:668 - TODO: Handle multiple levels. vertex=MethodCallExpressionVertex{fullMethodName=apiEnabledSessionId, referenceVertex=LazyVertex{result=EmptyReferenceExpression{properties={FirstChild=true, BeginLine=788, DefiningType_CaseSafe=xxxx_configurationwizardcontroller, LastChild=true, DefiningType=xxxx_ConfigurationWizardController, EndLine=788, childIdx=0, BeginColumn=43}}}, chainedNames=[], properties={FirstChild=false, FullMethodName=apiEnabledSessionId, BeginLine=788, FullMethodName_CaseSafe=apienabledsessionid, DefiningType_CaseSafe=xxxx_configurationwizardcontroller, LastChild=true, DefiningType=xxxx_ConfigurationWizardController, EndLine=788, MethodName_CaseSafe=apienabledsessionid, childIdx=1, BeginColumn=43, MethodName=apiEnabledSessionId}} , keys=[Ljava.lang.String;@4a667b2f

Also I noticed in the log that commented out lines where getting warnings against them. Does the scan look at commented out code too?

thanks for your assistance Henry

git2gus[bot] commented 1 year ago

This issue has been linked to a new work item: W-13189942

mtstapletonq commented 1 year ago

Did you ever find a resolution for this. We're having the same issue when we're getting the session ID from UserInfo.getSessionId(). When we use that value the scanner seems to get into a infinite loop until the scanner times out, or the machine runs out of memory.

We've had a number of these issues where infinite loops kick in for working normal code, and we have to change it just to get around the loop.

jfeingold35 commented 1 year ago

@mtstapletonq , what version of the scanner are you using? Also, do you have sample code you can share for us to reproduce with?

mtstapletonq commented 1 year ago

I'm using @salesforce/sfdx-scanner 3.15.0 (3.15.0) version of the scanner.

The line in question is: service.SessionHeader.sessionId = UserInfo.getSessionId(); And this results in the following looping infinitely in the sfge.log file: 2023-08-14 16:56:40 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN PathScopeVisitor:900 - TODO: Handle multiple levels. vertex=AssignmentExpression{properties={FirstChild=true, Operator==, BeginLine=158, DefiningType_CaseSafe=welcome, LastChild=true, DefiningType=Welcome, EndLine=158, childIdx=0, BeginColumn=13}} , lhs=VariableExpression{properties={FirstChild=true, BeginLine=158, DefiningType_CaseSafe=welcome, LastChild=false, DefiningType=Welcome, EndLine=158, Name_CaseSafe=sessionid, childIdx=0, BeginColumn=35, Name=sessionId}} 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished. 2023-08-14 16:56:41 2cec6cf9-e869-4ad2-a8f7-d3c2fc7d18ff WARN ApexPathExpanderUtil:233 - expand-Finished.

The process only ends if the path limit is reached, or if you have that turned off when you run out of memory.

Other examples which get stuck in infinite loops are: connectedApp.canvasConfig.locations = new List<String>{'Visualforce', 'Aura', 'PageLayout'}; with a work around of: connectedApp.canvasConfig.locations = 'Visualforce,Aura,PageLayout'.split(','); Not getting stuck in an infinite loop. Personally, I think the first version is much better code that the second one.