- Notifications
You must be signed in to change notification settings - Fork1.1k
chore: create interface for pkgs to return codersdk errors#18719
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Emyrk commentedJul 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
b43bd8e to15a6ca1Comparef0dd768 toa87678dCompare96c83c7 to8d594a7Compare
johnstcn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I only have nits about naming and usage conventions, but these aren't blocking.
coderd/dynamicparameters/error.go Outdated
| Diagnostics hcl.Diagnostics | ||
| KeyedDiagnosticsmap[string]hcl.Diagnostics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: What's the difference betweenDiagnostics andKeyedDiagnostics?
When should you use one or the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I will leave more comments!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Adds a unified error interface and refactors workspace‐build and parameter validation errors to produce structuredcodersdk.Response payloads.
- Introduce
CoderSDKErrorinterface andIsCoderSDKErrorhelper inhttperror/responserror.go - Implement
Response()onBuildErrorand createDiagnosticErrorfor dynamic parameters - Update HTTP error writer and tests to use the new interface
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| coderd/wsbuilder/wsbuilder_test.go | AddedTestWsbuildError to verifyBuildError meets the new interface |
| coderd/wsbuilder/wsbuilder.go | ImplementedResponse() onBuildError for structured responses |
| coderd/httpapi/httperror/wsbuild.go | Simplified error handling to useIsCoderSDKError |
| coderd/httpapi/httperror/responserror.go | DefinedCoderSDKError interface andIsCoderSDKError function |
| coderd/dynamicparameters/resolver.go | Swapped outResolverError for calls toParameterValidationError |
| coderd/dynamicparameters/error.go | AddedDiagnosticError type, constructors, and response logic |
Comments suppressed due to low confidence (3)
coderd/httpapi/httperror/responserror.go:9
- Add a doc comment above the
CoderSDKErrorinterface explaining its purpose and usage to improve discoverability.
type CoderSDKError interface {coderd/dynamicparameters/error.go:13
- Add unit tests for
ParameterValidationError(and the resultingDiagnosticError.Response) to ensure validation error formatting remains correct.
func ParameterValidationError(diags hcl.Diagnostics) *DiagnosticError {coderd/dynamicparameters/error.go:21
- Include doc comments for
TagValidationErrorand theDiagnosticErrortype to describe when and how these should be used.
func TagValidationError(diags hcl.Diagnostics) *DiagnosticError {Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
699dd8e intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
The dynamic parameter code lives in a package far removed from the httpapi and handlers. This interface allows it to create rich codersdk errors and pass them up to the
wsbuildererror handling.This interface should probably be used in more places. I got tired of making custom error handling code for unique types.