search

pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
194 stars 43 forks source link

Wrap calls to the upstream provider with a panic handler #1797

Open mjeffryes opened 6 months ago

mjeffryes commented 6 months ago

We found a decent number of bugs that are panics in the upstream provider: https://github.com/pulumi/pulumi-gcp/issues/1210 https://github.com/pulumi/pulumi-github/issues/323 https://github.com/pulumi/pulumi-cloudflare/issues/213 https://github.com/pulumi/pulumi-artifactory/issues/582

The ultimate fix for these could be submitting a fix to the upstream, or modifying the way we call the provider to avoid triggering the panic. In the meantime however, the panic is a rather jarring error for the user; wrapping these upstream calls with a panic handle would let us present a more user friendly error. (However, the user will likely still be blocked until we're able to fix the underlying problem.)

mjeffryes commented 6 months ago

A counter point could be that it's helpful to see the full panic trace when debugging these so presenting a friendlier error could mean less useful bug reports.

iwahbe commented 4 days ago

We do have panic handling for some functions in some cases:

https://github.com/pulumi/pulumi-terraform-bridge/blob/325b4f620bf51a8ee83bd9d9a6b0c2d621812a59/pkg/tfbridge/provider.go#L1318-L1327

Right now, panic handling is only turned on when we detect a type error.

t0yv0 commented 3 days ago

These are also indicative of bridge bugs when bridge sends input formatted differently from how TF sends it, triggering a panic. I'd be wary of burying these so we never look at them.