diff --git a/api_handler.go b/api_handler.go index be56f508..46c4ac74 100644 --- a/api_handler.go +++ b/api_handler.go @@ -42,9 +42,63 @@ type withSetBundle interface { SetBundle(bundle *apiBundle) } +type statusResponse struct { + Status string `json:"status"` +} + +var statusResponseOK = &statusResponse{Status: "ok"} //nolint:gochecknoglobals + +type healthCheckGetEndpoint struct { + apiBundle + apiendpoint.Endpoint[healthCheckGetRequest, statusResponse] +} + +func (*healthCheckGetEndpoint) Meta() *apiendpoint.EndpointMeta { + return &apiendpoint.EndpointMeta{ + Pattern: "GET /api/health-checks/{name}", + StatusCode: http.StatusOK, + } +} + +type healthCheckName string + +const ( + healthCheckNameComplete healthCheckName = "complete" + healthCheckNameMinimal healthCheckName = "minimal" +) + +type healthCheckGetRequest struct { + Name healthCheckName `json:"-"` // from ExtractRaw +} + +func (req *healthCheckGetRequest) ExtractRaw(r *http.Request) error { + req.Name = healthCheckName(r.PathValue("name")) + return nil +} + +func (a *healthCheckGetEndpoint) Execute(ctx context.Context, req *healthCheckGetRequest) (*statusResponse, error) { + switch req.Name { + case healthCheckNameComplete: + if _, err := a.dbPool.Exec(ctx, "SELECT 1"); err != nil { + return nil, apierror.WithInternalError( + apierror.NewServiceUnavailable("Unable to query database. Check logs for details."), + err, + ) + } + + case healthCheckNameMinimal: + // fall through to OK status response below + + default: + return nil, apierror.NewNotFound("Health check %q not found. Use either `complete` or `minimal`.", req.Name) + } + + return statusResponseOK, nil +} + type jobCancelEndpoint struct { apiBundle - apiendpoint.Endpoint[jobCancelRequest, jobCancelResponse] + apiendpoint.Endpoint[jobCancelRequest, statusResponse] } func (*jobCancelEndpoint) Meta() *apiendpoint.EndpointMeta { @@ -58,12 +112,8 @@ type jobCancelRequest struct { JobIDs []int64String `json:"ids"` } -type jobCancelResponse struct { - Status string `json:"status"` -} - -func (a *jobCancelEndpoint) Execute(ctx context.Context, req *jobCancelRequest) (*jobCancelResponse, error) { - return dbutil.WithTxV(ctx, a.dbPool, func(ctx context.Context, tx pgx.Tx) (*jobCancelResponse, error) { +func (a *jobCancelEndpoint) Execute(ctx context.Context, req *jobCancelRequest) (*statusResponse, error) { + return dbutil.WithTxV(ctx, a.dbPool, func(ctx context.Context, tx pgx.Tx) (*statusResponse, error) { updatedJobs := make(map[int64]*rivertype.JobRow) for _, jobID := range req.JobIDs { jobID := int64(jobID) @@ -78,7 +128,7 @@ func (a *jobCancelEndpoint) Execute(ctx context.Context, req *jobCancelRequest) } // TODO: return jobs in response, use in frontend instead of invalidating - return &jobCancelResponse{Status: "ok"}, nil + return statusResponseOK, nil }) } diff --git a/api_handler_test.go b/api_handler_test.go index 61043128..9e55262c 100644 --- a/api_handler_test.go +++ b/api_handler_test.go @@ -44,7 +44,57 @@ func setupEndpoint[TEndpoint any](ctx context.Context, t *testing.T) (*TEndpoint } } -func TestAPIHandlerJobCancel(t *testing.T) { +func TestHandlerHealthCheckGetEndpoint(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + t.Run("CompleteSuccess", func(t *testing.T) { + t.Parallel() + + endpoint, _ := setupEndpoint[healthCheckGetEndpoint](ctx, t) + + resp, err := endpoint.Execute(ctx, &healthCheckGetRequest{Name: healthCheckNameComplete}) + require.NoError(t, err) + require.Equal(t, statusResponseOK, resp) + }) + + t.Run("CompleteDatabaseError", func(t *testing.T) { + t.Parallel() + + endpoint, bundle := setupEndpoint[healthCheckGetEndpoint](ctx, t) + + // Roll back prematurely so we get a database error. + require.NoError(t, bundle.tx.Rollback(ctx)) + + _, err := endpoint.Execute(ctx, &healthCheckGetRequest{Name: healthCheckNameComplete}) + requireAPIError(t, apierror.WithInternalError( + apierror.NewServiceUnavailable("Unable to query database. Check logs for details."), + pgx.ErrTxClosed, + ), err) + }) + + t.Run("Minimal", func(t *testing.T) { + t.Parallel() + + endpoint, _ := setupEndpoint[healthCheckGetEndpoint](ctx, t) + + resp, err := endpoint.Execute(ctx, &healthCheckGetRequest{Name: healthCheckNameMinimal}) + require.NoError(t, err) + require.Equal(t, statusResponseOK, resp) + }) + + t.Run("NotFound", func(t *testing.T) { + t.Parallel() + + endpoint, _ := setupEndpoint[healthCheckGetEndpoint](ctx, t) + + _, err := endpoint.Execute(ctx, &healthCheckGetRequest{Name: "other"}) + requireAPIError(t, apierror.NewNotFound("Health check %q not found. Use either `complete` or `minimal`.", "other"), err) + }) +} + +func TestJobCancelEndpoint(t *testing.T) { t.Parallel() ctx := context.Background() @@ -62,7 +112,7 @@ func TestAPIHandlerJobCancel(t *testing.T) { resp, err := endpoint.Execute(ctx, &jobCancelRequest{JobIDs: []int64String{int64String(insertRes1.Job.ID), int64String(insertRes2.Job.ID)}}) require.NoError(t, err) - require.Equal(t, &jobCancelResponse{Status: "ok"}, resp) + require.Equal(t, statusResponseOK, resp) updatedJob1, err := bundle.client.JobGetTx(ctx, bundle.tx, insertRes1.Job.ID) require.NoError(t, err) @@ -83,7 +133,7 @@ func TestAPIHandlerJobCancel(t *testing.T) { }) } -func TestAPIHandlerJobGet(t *testing.T) { +func TestJobGetEndpoint(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/handler.go b/handler.go index 6d460525..00a2c08e 100644 --- a/handler.go +++ b/handler.go @@ -88,6 +88,7 @@ func NewHandler(opts *HandlerOpts) (http.Handler, error) { prefix := opts.Prefix mux := http.NewServeMux() + apiendpoint.Mount(mux, opts.Logger, &healthCheckGetEndpoint{apiBundle: apiBundle}) mux.HandleFunc("GET /api/jobs", handler.JobList) apiendpoint.Mount(mux, opts.Logger, &jobCancelEndpoint{apiBundle: apiBundle}) mux.HandleFunc("POST /api/jobs/delete", handler.JobDelete) diff --git a/handler_test.go b/handler_test.go index ac49b7b8..6f855a99 100644 --- a/handler_test.go +++ b/handler_test.go @@ -88,6 +88,8 @@ func TestNewHandlerIntegration(t *testing.T) { // API calls // + makeAPICall(t, "HealthCheckGetComplete", http.MethodGet, makeURL("/api/health-checks/%s", healthCheckNameComplete), nil) + makeAPICall(t, "HealthCheckGetMinimal", http.MethodGet, makeURL("/api/health-checks/%s", healthCheckNameMinimal), nil) makeAPICall(t, "JobCancel", http.MethodPost, makeURL("/api/jobs/cancel"), mustMarshalJSON(t, &jobCancelRequest{JobIDs: []int64String{int64String(job.ID)}})) makeAPICall(t, "JobGet", http.MethodGet, makeURL("/api/jobs/%d", job.ID), nil) } diff --git a/internal/apiendpoint/api_endpoint.go b/internal/apiendpoint/api_endpoint.go index 52288e0c..9bbe4e3e 100644 --- a/internal/apiendpoint/api_endpoint.go +++ b/internal/apiendpoint/api_endpoint.go @@ -141,8 +141,17 @@ func executeAPIEndpoint[TReq any, TResp any](w http.ResponseWriter, r *http.Requ if err != nil { var apiErr apierror.Interface if errors.As(err, &apiErr) { + logAttrs := []any{ + slog.String("error", apiErr.Error()), + } + + if internalErr := apiErr.GetInternalError(); internalErr != nil { + logAttrs = append(logAttrs, slog.String("internal_error", internalErr.Error())) + } + // Logged at info level because API errors are normal. - logger.InfoContext(ctx, "API error response", slog.String("error", apiErr.Error())) + logger.InfoContext(ctx, "API error response", logAttrs...) + apiErr.Write(ctx, logger, w) return } diff --git a/internal/apierror/api_error.go b/internal/apierror/api_error.go index ac7bc257..ac2c9f5a 100644 --- a/internal/apierror/api_error.go +++ b/internal/apierror/api_error.go @@ -16,6 +16,11 @@ import ( // // APIErrorInterface should be used with errors.As instead of this struct. type APIError struct { + // InternalError is an additional error that might be associated with the + // API error. It's not returned in the API error response, but is logged in + // API endpoint execution to provide extra information for operators. + InternalError error `json:"-"` + // Message is a descriptive, human-friendly message indicating what went // wrong. Try to make error messages as actionable as possible to help the // caller easily fix what went wrong. @@ -26,7 +31,9 @@ type APIError struct { StatusCode int `json:"-"` } -func (e *APIError) Error() string { return e.Message } +func (e *APIError) Error() string { return e.Message } +func (e *APIError) GetInternalError() error { return e.InternalError } +func (e *APIError) SetInternalError(internalErr error) { e.InternalError = internalErr } // Write writes the API error to an HTTP response, writing to the given logger // in case of a problem. @@ -48,9 +55,18 @@ func (e *APIError) Write(ctx context.Context, logger *slog.Logger, w http.Respon // won't be usable as an errors.As target. type Interface interface { Error() string + GetInternalError() error + SetInternalError(internalErr error) Write(ctx context.Context, logger *slog.Logger, w http.ResponseWriter) } +// WithInternalError is a convenience function for assigning an internal error +// to the given API error and returning it. +func WithInternalError[TAPIError Interface](apiErr TAPIError, internalErr error) TAPIError { + apiErr.SetInternalError(internalErr) + return apiErr +} + // // BadRequest // diff --git a/internal/apierror/api_error_test.go b/internal/apierror/api_error_test.go index 060e8b8a..6a61cb2c 100644 --- a/internal/apierror/api_error_test.go +++ b/internal/apierror/api_error_test.go @@ -3,6 +3,7 @@ package apierror import ( "context" "encoding/json" + "errors" "net/http/httptest" "testing" @@ -11,6 +12,18 @@ import ( "github.com/riverqueue/riverui/internal/riverinternaltest" ) +func TestAPIError(t *testing.T) { + t.Parallel() + + var ( + anErr = errors.New("an error") + apiErr = NewBadRequest("Bad request.") + ) + + apiErr.SetInternalError(anErr) + require.Equal(t, anErr, apiErr.GetInternalError()) +} + func TestAPIErrorJSON(t *testing.T) { t.Parallel() @@ -40,6 +53,18 @@ func TestAPIErrorWrite(t *testing.T) { ) } +func TestWithInternalError(t *testing.T) { + t.Parallel() + + var ( + anErr = errors.New("an error") + apiErr = NewBadRequest("Bad request.") + ) + + apiErr = WithInternalError(apiErr, anErr) + require.Equal(t, anErr, apiErr.InternalError) +} + func mustMarshalJSON(t *testing.T, v any) []byte { t.Helper()