Expose SignalFx 4xx API error details across provider#682
Expose SignalFx 4xx API error details across provider#682tnoff wants to merge 2 commits intosplunk-terraform:mainfrom
Conversation
- Centralize diagnostic surfacing in tfext.AsErrorDiagnostics for ResponseError (adds Details to diagnostics) - Add common.WrapResponseError and use it in *_chart resources with common.HandleError to preserve 404 state clearing - Update tests and helpers to accept Detail field while keeping Summary stable Droid-assisted
|
Hey @tnoff, Thank you for your contribution :D I apologise because my attention is elsewhere for the moment, and I'd like to give this the attention it deserves. For the time being I'll run the pipeline to perform the simple validation. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 94.38% 94.29% -0.09%
==========================================
Files 77 77
Lines 3813 3842 +29
==========================================
+ Hits 3599 3623 +24
- Misses 143 145 +2
- Partials 71 74 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Run lint --fix to resolve gci import formatting in internal/common/error.go - Add tests for tfext AsErrorDiagnostics handling of ResponseError (direct and wrapped) - Add tests for common.WrapResponseError preserving identity/message All linters pass; unit tests green with coverage generated. Droid-assisted
|
@MovieStoreGuy thank you! I just added some updates to hopefully address the issues it found in the original commit |
|
Have not forgotten about this sorry, just thought these errors were already expanded as part of the decorators. Let me double check why that isn't the case. |
|
Hey @tnoff , I had recently added https://github.com/splunk-terraform/terraform-provider-signalfx/blob/main/signalfx/provider_decorator.go#L37:L53 into the provider that should be doing the same thing as you're attempting to do. Is this not working for you? |
|
@MovieStoreGuy ah lemme try that, I believe we're on 9.7.2, lemme update to 9.21.0 per d113862 |
|
Was there any updates @tnoff ? |
Issue: When creating/updating chart resources, 400 responses do not show the response text to show the exact error of why something failed
Workaround: Currently if you run the terraform command with
TF_LOG=debugin the front to show the full response back from the API the error is shownProposal: Expose the 4xx api details for charts