Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.22k stars 747 forks source link

Importing types from a common module causes `An item with the same key has already been added` exception #14196

Closed miqm closed 2 months ago

miqm commented 4 months ago

Bicep version v0.27.1

Describe the bug Module A exports a type. Modules B and C imports that type (crucial - under this same names!) and as a part of it's own type - type from module A is exported Main file includes types from B and C Compilation fails

To Reproduce moduleA.bicep

@export()
type typeA = {
 propA: string
}

moduleB.bicep

import * as typesA from 'moduleA.bicep'
@export()
type typeB = {
 optionsA: typesA.typeA
 propB: string
}

moduleC.bicep

import * as typesA from 'moduleA.bicep'
@export()
type typeB = {
 optionsA: typesA.typeA
 propC: string
}

main.bicep

import * as typesB from 'moduleB.bicep'
import * as typesC from 'moduleC.bicep'

Additional context This problem does not exist if imports in moduleB and moduleC have same syntax, i.e. import * as typesA from 'moduleA.bicep'

stacktrace

Internal Error - System.ArgumentException: An item with the same key has already been added. Key: _1.typeA
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at Bicep.Core.Emit.CompileTimeImports.ImportClosureInfo.Calculate(SemanticModel model) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\CompileTimeImports\ImportClosureInfo.cs:line 105
   at Bicep.Core.Emit.EmitterContext.<>c__DisplayClass1_0.<.ctor>b__0() in C:\__w\1\s\bicep\src\Bicep.Core\Emit\EmitterContext.cs:line 25
   at System.Lazy`1.PublicationOnlyViaFactory(LazyHelper initializer)
   at System.Lazy`1.CreateValue()
   at Bicep.Core.Emit.EmitterContext.get_ImportClosureInfo() in C:\__w\1\s\bicep\src\Bicep.Core\Emit\EmitterContext.cs:line 44
   at Bicep.Core.Emit.TemplateWriter.GenerateTemplateWithoutHash(PositionTrackingJsonTextWriter jsonWriter) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateWriter.cs:line 103
   at Bicep.Core.Emit.TemplateWriter.Write(SourceAwareJsonTextWriter writer) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateWriter.cs:line 81
   at Bicep.Core.Emit.TemplateEmitter.<>c__DisplayClass8_0.<Emit>b__0() in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateEmitter.cs:line 114
   at Bicep.Core.Emit.TemplateEmitter.EmitOrFail(Func`1 write) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateEmitter.cs:line 135
   at Bicep.Core.Emit.TemplateEmitter.Emit(TextWriter textWriter) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateEmitter.cs:line 100
   at Bicep.Core.Emit.TemplateEmitter.Emit(Stream stream) in C:\__w\1\s\bicep\src\Bicep.Core\Emit\TemplateEmitter.cs:line 93
   at Bicep.LanguageServer.Handlers.BicepBuildCommandHandler.GenerateCompiledFileAndReturnBuildOutputMessage(String bicepFilePath, DocumentUri documentUri) in C:\__w\1\s\bicep\src\Bicep.LangServer\Handlers\BicepBuildCommandHandler.cs:line 71
   at Bicep.LanguageServer.Handlers.BicepBuildCommandHandler.Handle(String bicepFilePath, CancellationToken cancellationToken) in C:\__w\1\s\bicep\src\Bicep.LangServer\Handlers\BicepBuildCommandHandler.cs:line 42
   at OmniSharp.Extensions.LanguageServer.Server.Pipelines.SemanticTokensDeltaPipeline`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at OmniSharp.Extensions.LanguageServer.Server.Pipelines.ResolveCommandPipeline`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestPreProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestPostProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestExceptionProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestExceptionProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestExceptionActionProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at MediatR.Pipeline.RequestExceptionActionProcessorBehavior`2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate`1 next)
   at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.<RouteRequest>g__InnerRoute|7_0(IServiceScopeFactory serviceScopeFactory, Request request, TDescriptor descriptor, Object params, CancellationToken token, ILogger logger)
   at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.RouteRequest(IRequestDescriptor`1 descriptors, Request request, CancellationToken token)
   at OmniSharp.Extensions.JsonRpc.DefaultRequestInvoker.<>c__DisplayClass10_0.<<RouteRequest>b__5>d.MoveNext()
jeskew commented 3 months ago

@miqm -- Did this error occur only once, or is it regularly reproducible for you? I have a fix but am a bit worried that I'm missing something since I haven't been able to reproduce using the provided templates.

miqm commented 3 months ago

It was reproducible as in the issue.

jeskew commented 3 months ago

Are you only seeing it in certain environments? The following test written from the repro case is passing every time in a Windows desktop environment:

    [TestMethod]
    public void Symbols_entering_the_import_closure_via_multiple_paths_are_supported()
    {
        var result = CompilationHelper.Compile(
            ("main.bicep", """
                import * as typesB from 'moduleB.bicep'
                import * as typesC from 'moduleC.bicep'
                """),
            ("moduleA.bicep", """
                @export()
                type typeA = {
                    propA: string
                }
                """),
            ("moduleB.bicep", """
                import * as typesAinB from 'moduleA.bicep'
                @export()
                type typeB = {
                    optionsA: typesAinB.typeA
                    propB: string
                }
                """),
            ("moduleC.bicep", """
                import * as typesAinC from 'moduleA.bicep'
                @export()
                type typeB = {
                    optionsA: typesAinC.typeA
                    propC: string
                }
                """));

        result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
    }
miqm commented 3 months ago

I tried to reproduce it on Windows and I couldn't. however I mainly do bicep work on Mac (M1) so it could be platform-related...

miqm commented 3 months ago

Wait, I have a repro - the names of import as in modB and modC must be equal, not different. it works when they are different, but fails when there are equal. I probably got confused when writing the bug report. sorry.... I updated the issue.

jeskew commented 3 months ago

Is that failure also only on a Mac? It seems to be working fine on Windows, but the logic to deduplicate looks like it's relying on some default equality checks. Those shouldn't differ from platform to platform, but I've seen similar issues in the past.

miqm commented 3 months ago

The repro I have it's on Windows, please check where instead import * as typesAinB/C from 'moduleA.bicep' you have import * as typesA from 'moduleA.bicep' in both modB and modC

jeskew commented 3 months ago

I am able to reproduce on Windows, but only when building main.bicep from the VS Code UI. (Building the same files with bicep build main.bicep succeeds without an error.) Does this match your experience on a Mac?

If so, I think the difference is not related to platform but rather caused by how Bicep caches compiled templates. The code that's throwing an error assumes that moduleA.bicep in the example will always resolve to the same in-memory representation. This appears to be true for the Bicep CLI (and for the unit tests) but may not necessarily be the case for the VS Code extension.

miqm commented 3 months ago

Not sure now, but it definitely was (and is) failing when building from the VSCode. Now that I test, from cli it seems to be fine.

miqm commented 2 months ago

@jeskew any findings?

jeskew commented 2 months ago

@miqm From what I have been able to find, this error can only occur when the following conditions are met:

  1. An ARM template is being compiled by the language server
  2. A symbol is imported separately at multiple points within a compilation closure (e.g., typeA -> moduleB.bicep and typeA -> moduleC.bicep)
  3. Multiple such points are active (e.g., main.bicep and moduleB.bicep are loaded as tabs in VS Code)
  4. The symbol's source is not active (e.g., moduleA.bicep is not open in VS Code)
  5. A file in which the symbol is imported other than the primary compilation target is altered (e.g., moduleB.bicep is updated in VS Code)

The root cause is that the language server stores an "active context" for each open .bicep or .bicepparam tab in VS Code, and each active context will retain a compiled model of each file within the tab's transitive closure (i.e., any Bicep files used as either modules or import sources, plus files they use as modules or import sources, etc.). Following the sequence of steps outlined above can result in the VS Code extension combining models from different contexts.

The logic that handles transitive imports was assuming every compiled model would be sourced from the same compilation context. I opened a PR to have that logic deduplicate imports based on the identifiers of the imported symbol rather than using object identity as a basis for comparison.