bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Fatal linter error in build

Open StephenWeatherford opened this issue 3 years ago • 2 comments
trafficstars

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Failed Modules_should_have_metadata [32 ms]
  Error Message:
   Expected collection to be empty, but found {"[linter-internal-error (Warning)] Analyzer 'core/explicit-values-for-loc-params' encountered an unexpected exception. Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.", "[linter-internal-error (Warning)] Analyzer 'core/explicit-values-for-loc-params' encountered an unexpected exception. Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct."}.
  Stack Trace:
     at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.GivenSelector`1.FailWith(String message, Object[] args)
   at FluentAssertions.Collections.GenericCollectionAssertions`3.BeEmpty(String because, Object[] becauseArgs)
   at Bicep.Core.UnitTests.Assertions.IDiagnosticCollectionAssertions.BeEmpty(String because, Object[] becauseArgs)
   at Bicep.Core.IntegrationTests.ModuleTests.Modules_should_have_metadata() in /home/runner/work/bicep/bicep/src/Bicep.Core.IntegrationTests/ModuleTests.cs:line 319

https://github.com/Azure/bicep/runs/7676341077?check_suite_focus=true

ALSO: [linter-internal-error (Warning)] Analyzer 'core/no-hardcoded-location' encountered an unexpected exception. Object reference not set to an instance of an object."

StephenWeatherford avatar Aug 04 '22 17:08 StephenWeatherford

Some thoughts:

  1. Should we just get rid of the code to catch linter exceptions and return a diagnostic for it? I feel like it would be better to fail explicitly so that we are notified of problems and they're easier to troubleshoot from logs:
    • Here: https://github.com/Azure/bicep/blob/2b41fcfbe56b6650b99bb8a46eabe32ff8f3d529/src/Bicep.Core/Analyzers/Linter/LinterAnalyzer.cs#L62-L66
    • And here: https://github.com/Azure/bicep/blob/2b41fcfbe56b6650b99bb8a46eabe32ff8f3d529/src/Bicep.Core/Analyzers/Linter/LinterRuleBase.cs#L92-L97
  2. Linters are reused and can be scheduled concurrently, but the code doesn't appear to expect that. It's not possible to confirm without the stack trace, but I suspect the problem occurred here with the _paramsUsedInLocationPropsForFile non-concurrent dictionary: https://github.com/Azure/bicep/blob/2b41fcfbe56b6650b99bb8a46eabe32ff8f3d529/src/Bicep.Core/Analyzers/Linter/Rules/LocationRuleBase.cs#L188-L196 It's probably worth either avoiding caching entirely (unless we're sure there's going to be a perf problem), or coming up with a standardized reusable pattern for caching.

anthony-c-martin avatar Aug 05 '22 16:08 anthony-c-martin

Good point on both. Frankly not sure what I was thinking on the caching - it should obviously be limited to a single analysis call.

StephenWeatherford avatar Aug 05 '22 18:08 StephenWeatherford