Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve type-checker errors #1986

Closed
iwahbe opened this issue May 17, 2024 · 0 comments 路 Fixed by #1997
Closed

Improve type-checker errors #1986

iwahbe opened this issue May 17, 2024 · 0 comments 路 Fixed by #1997
Assignees
@corymhall
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed

Comments

@iwahbe
Copy link
Member

iwahbe commented May 17, 2024 •
edited

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Currently, type checking failures are displayed like this:

warning: Type checking failed. If any of these are incorrect, please let us know by creating an issue at https://github.com/pului/pulumi-terraform-bridge/issues.
verification warning: expected object type, got [] type: Examine values at environmentVariables

(source: #1979)

If the bridged provider panics (which is likely for a genuine type checker failure, the user sees this:

Full Output 
warning: Type checking failed. If any of these are incorrect, please let us know by creating an issue at https://github.com/pului/pulumi-terraform-bridge/issues.
verification warning: expected object type, got [] type: Examine values at environmentVariables

 pulumi:pulumi:Stack (quill-infrastructure-production):
    panic: value has no attribute of that name
    goroutine 1332 [running]:
    github.com/zclconf/go-cty/cty.Value.GetAttr({{{0x5282960?, 0xc006ca5310?}}, {0x4231d80?, 0xc006cb8ff0?}}, {0x4990389, 0x7})
    	/home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.2/cty/value_ops.go:808 +0x2f1
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2/internal/tf/plans/objchange.proposedNewAttributes(0xc006cb82d0, {{{0x5282960?, 0xc006ca50e0?}}, {0x4231d80?, 0xc006cb8bd0?}}, {{{0x5282960?, 0xc006ca5310?}}, {0x4231d80?, 0xc006cb8ff0?}})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/internal/tf/plans/objchange/objchange.go:289 +0x231
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2/internal/tf/plans/objchange.proposedNew(0xc006cb82a0, {{{0x5282960?, 0xc006ca50e0?}}, {0x4231d80?, 0xc006cb8bd0?}}, {{{0x5282960?, 0xc006ca5310?}}, {0x4231d80?, 0xc006cb8ff0?}})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/internal/tf/plans/objchange/objchange.go:89 +0x15b
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2/internal/tf/plans/objchange.ProposedNew(0x5282730?, {{{0x5282960?, 0xc006ca50e0?}}, {0x4231d80?, 0xc006cb8bd0?}}, {{{0x5282960?, 0xc006ca5310?}}, {0x4231d80?, 0xc006cb8ff0?}})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/internal/tf/plans/objchange/objchange.go:46 +0x139
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.proposedNew(0xc006c9e240?, {{{0x5282730?, 0xc006903b18?}}, {0x4231d80?, 0xc006c9f4d0?}}, {{{0x5282730?, 0xc006903248?}}, {0x4231d80?, 0xc006a3bef0?}})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/proposed_new.go:34 +0x156
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.simpleDiffViaPlanState({0x5281f68, 0xc006a3ac60}, 0x4231d80?, 0xc006c59380?, {{{0x5282730?, 0xc006903248?}}, {0x4231d80?, 0xc006a3bef0?}}, {0x494cc60, 0xc0050a4c00})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/provider_diff.go:176 +0xff
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.v2Provider.simpleDiff({0x4167bf?, {0x4231d80?, 0xc006ba9e60?, 0xc006a60b40?}}, {0x5281f68, 0xc006a3ac60}, 0x416050?, 0x0?, 0xc006ba9e60?, 0xc006a31400, ...)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/provider_diff.go:125 +0x698
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.v2Provider.Diff({0xc000727b00?, {0xc00042b6c0?, 0xc006ba9e60?, 0xc005289228?}}, {0x5281f68, 0xc006a3ac60}, {0x4a00eab, 0x1e}, {0x52827a0, 0xc006a20f90}, ...)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/provider_diff.go:75 +0x406
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.(*providerWithPlanResourceChangeDispatch).Diff(0xc00113bb90, {0x5281f68, 0xc006a3ac60}, {0x4a00eab, 0x1e}, {0x52827a0, 0xc006a20f90}, {0x525b6a0, 0xc006a31400}, {0x0, ...})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfshim/sdk-v2/provider2.go:690 +0x162
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.(*Provider).Diff(0xc00002cc80, {0x5281f68?, 0xc006a3a840?}, 0xc006960880)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.82.0/pkg/tfbridge/provider.go:946 +0x935
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Diff.func1({0x529a9e0?, 0xc00002cc80?})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.9-0.20240227144008-2da15b3d6f6e/muxer.go:369 +0x30
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.resourceMethod[...](0xc004f61560?, 0x40, 0xc0052896c0?)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.9-0.20240227144008-2da15b3d6f6e/muxer.go:334 +0xb5
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Diff(0x0?, {0x5281f68?, 0xc006a3a840?}, 0x40?)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.9-0.20240227144008-2da15b3d6f6e/muxer.go:368 +0x65
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler.func1({0x5281f68, 0xc006a3a840}, {0x4761f20?, 0xc006960880})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.114.0/proto/go/provider_grpc.pb.go:575 +0x72
    github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0x5281f68, 0xc006a3a1e0}, {0x4761f20, 0xc006960880}, 0xc006a36100, 0xc006a20d80)
    	/home/runner/go/pkg/mod/github.com/grpc-ecosystem/grpc-opentracing@v0.0.0-20180507213350-8e809c8a8645/go/otgrpc/server.go:57 +0x3d0
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler({0x4907300?, 0xc004f61560}, {0x5281f68, 0xc006a3a1e0}, 0xc006960800, 0xc001186700)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.114.0/proto/go/provider_grpc.pb.go:577 +0x135
    google.golang.org/grpc.(*Server).processUnaryRPC(0xc00021ec00, {0x5281f68, 0xc006a3a150}, {0x5292300, 0xc00064f080}, 0xc006a38360, 0xc004fe88d0, 0x95d0588, 0x0)
    	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1369 +0xe23
    google.golang.org/grpc.(*Server).handleStream(0xc00021ec00, {0x5292300, 0xc00064f080}, 0xc006a38360)
    	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1780 +0x1016
    google.golang.org/grpc.(*Server).serveStreams.func2.1()
    	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1019 +0x8b
    created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 53
    	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1030 +0x135

We can do better on the error message. I'm imagining something like this:

warning: type checking failed:
    Unexpected type at field 'environmentVariables':
      Expected 'object' type
      Got '[]' type

Type checking is still experimental. If you believe that a warning is incorrect, 
please let us know by creating an issue at https://github.com/pulumi/pulumi-terraform-bridge/issues.
This will become a hard error in the future.

Improvements we need to make:

  • The warning should be big and have plenty of whitespace. It should be hard to miss and not look crowded.
  • It should quote type names and field names. If we have the facilities, we could bold them as well.
  • It should mention that type-checking is experimental.
  • It should mention that the warning will become a hard error in the future.

Affected area/feature
@iwahbe iwahbe added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features labels May 17, 2024
@iwahbe iwahbe mentioned this issue May 17, 2024
Closed
@corymhall corymhall self-assigned this May 17, 2024
@corymhall corymhall removed the needs-triage Needs attention from the triage team label May 17, 2024
corymhall added a commit that referenced this issue May 20, 2024
@corymhall
chore: better type checking error messages
82ef773 
This is what the error message looks like

```console
  aws:ecs:Service (service):
    warning: Type checking failed:
      Unexpected type at field networkConfiguration.subnets:
        expected array type, got string type
    warning: Type checking is still experimental. If you believe that a warning is incorrect,
    please let us know by creating an issue at https://github.com/pulumi/pulumi-terraform-bridge/issues.
    This will become a hard error in the future.
```

fixes #1986
@corymhall corymhall mentioned this issue May 20, 2024
Merged
corymhall added a commit that referenced this issue May 22, 2024
@corymhall @iwahbe
chore: better type checking error messages (#1997)
eeb785e 
This is what the error message looks like

```console
  aws:ecs:Service (service):
    warning: Type checking failed:
      Unexpected type at field networkConfiguration.subnets:
        expected array type, got string type
    warning: Type checking is still experimental. If you believe that a warning is incorrect,
    please let us know by creating an issue at https://github.com/pulumi/pulumi-terraform-bridge/issues.
    This will become a hard error in the future.
```

fixes #1986

---------

Co-authored-by: Ian Wahbe <ian@wahbe.com>
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@corymhall corymhall

Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: better type checking error messages pulumi/pulumi-terraform-bridge
3 participants
@iwahbe @pulumi-bot @corymhall