Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 3, 2025

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jul 2, 2025

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 wsbuilder error handling.

This interface should probably be used in more places. I got tired of making custom error handling code for unique types.

Copy link
Member Author

Emyrk commented Jul 2, 2025

@Emyrk Emyrk changed the title chore: interface for pkgs to return codersdk errors chore: create interface for pkgs to return codersdk errors Jul 2, 2025
@Emyrk Emyrk force-pushed the stevenmasley/preview_errors branch 2 times, most recently from b43bd8e to 15a6ca1 Compare July 2, 2025 15:54
@Emyrk Emyrk force-pushed the stevenmasley/preview_errors branch 2 times, most recently from f0dd768 to a87678d Compare July 2, 2025 16:48
@Emyrk Emyrk marked this pull request as ready for review July 2, 2025 17:30
@Emyrk Emyrk requested a review from johnstcn July 2, 2025 21:44
@Emyrk Emyrk force-pushed the stevenmasley/preview_errors branch from 96c83c7 to 8d594a7 Compare July 2, 2025 21:51
Copy link
Member

@johnstcn johnstcn left a 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.

Comment on lines 31 to 32
Diagnostics hcl.Diagnostics
KeyedDiagnostics map[string]hcl.Diagnostics
Copy link
Member

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 between Diagnostics and KeyedDiagnostics?
When should you use one or the other?

Copy link
Member Author

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!

@Emyrk Emyrk requested a review from Copilot July 3, 2025 12:10
Copy link
Contributor

@Copilot Copilot AI left a 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 structured codersdk.Response payloads.

  • Introduce CoderSDKError interface and IsCoderSDKError helper in httperror/responserror.go
  • Implement Response() on BuildError and create DiagnosticError for 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 Added TestWsbuildError to verify BuildError meets the new interface
coderd/wsbuilder/wsbuilder.go Implemented Response() on BuildError for structured responses
coderd/httpapi/httperror/wsbuild.go Simplified error handling to use IsCoderSDKError
coderd/httpapi/httperror/responserror.go Defined CoderSDKError interface and IsCoderSDKError function
coderd/dynamicparameters/resolver.go Swapped out ResolverError for calls to ParameterValidationError
coderd/dynamicparameters/error.go Added DiagnosticError type, constructors, and response logic
Comments suppressed due to low confidence (3)

coderd/httpapi/httperror/responserror.go:9

  • Add a doc comment above the CoderSDKError interface explaining its purpose and usage to improve discoverability.
type CoderSDKError interface {

coderd/dynamicparameters/error.go:13

  • Add unit tests for ParameterValidationError (and the resulting DiagnosticError.Response) to ensure validation error formatting remains correct.
func ParameterValidationError(diags hcl.Diagnostics) *DiagnosticError {

coderd/dynamicparameters/error.go:21

  • Include doc comments for TagValidationError and the DiagnosticError type to describe when and how these should be used.
func TagValidationError(diags hcl.Diagnostics) *DiagnosticError {

@Emyrk Emyrk merged commit 699dd8e into main Jul 3, 2025
36 checks passed
@Emyrk Emyrk deleted the stevenmasley/preview_errors branch July 3, 2025 13:33
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants