Skip to content

Commit

Permalink
webui error handling + openapi gen update (#4413)
Browse files Browse the repository at this point in the history
This PR improves API error handling in the WebUI as well as adding
connection status indicator to tell if the backend (orchestrator node)
is alive and reachable or not.

In addition to that:
- Migrate OpenAPI typescript code generator library to a maintainable
one as the existing one is deprecated
- Fixes echo custom error handler to reflect correct error code, and
serializing RequestID and Status in the APIError in addition to the
headers to simplify error handling in non-go clients
  • Loading branch information
wdbaruni authored Sep 16, 2024
1 parent 9f7486e commit 01be858
Show file tree
Hide file tree
Showing 149 changed files with 1,724 additions and 5,858 deletions.
10 changes: 9 additions & 1 deletion docs/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"200": {
"description": "OK",
"schema": {
"type": "string"
"$ref": "#/definitions/apimodels.IsAliveResponse"
}
}
}
Expand Down Expand Up @@ -1045,6 +1045,14 @@
}
}
},
"apimodels.IsAliveResponse": {
"type": "object",
"properties": {
"Status": {
"type": "string"
}
}
},
"apimodels.ListJobExecutionsResponse": {
"type": "object",
"properties": {
Expand Down
8 changes: 5 additions & 3 deletions pkg/jobstore/boltdb/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/bacalhau-project/bacalhau/pkg/lib/marshaller"
"github.com/bacalhau-project/bacalhau/pkg/lib/math"
"github.com/bacalhau-project/bacalhau/pkg/models"
"github.com/bacalhau-project/bacalhau/pkg/publicapi/apimodels"
"github.com/bacalhau-project/bacalhau/pkg/util"
"github.com/bacalhau-project/bacalhau/pkg/util/idgen"
)
Expand Down Expand Up @@ -179,7 +178,7 @@ func (b *BoltJobStore) getJob(tx *bolt.Tx, jobID string) (models.Job, error) {

data := GetBucketData(tx, NewBucketPath(BucketJobs, jobID), SpecKey)
if data == nil {
return job, apimodels.NewJobNotFound(jobID)
return job, jobstore.NewErrJobNotFound(jobID)
}

err = b.marshaller.Unmarshal(data, &job)
Expand Down Expand Up @@ -912,7 +911,10 @@ func (b *BoltJobStore) deleteJob(tx *bolt.Tx, jobID string) error {

job, err := b.getJob(tx, jobID)
if err != nil {
apimodels.NewJobNotFound(jobID)
if models.IsBaseError(err) {
return err
}
return NewBoltDbError(err)
}

tx.OnCommit(func() {
Expand Down
15 changes: 14 additions & 1 deletion pkg/models/error.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package models

import (
"errors"
"fmt"
"net/http"
)
Expand Down Expand Up @@ -91,6 +92,13 @@ type BaseError struct {
code ErrorCode
}

// IsBaseError is a helper function that checks if an error is a BaseError.
func IsBaseError(err error) bool {
var baseError *BaseError
ok := errors.As(err, &baseError)
return ok
}

// NewBaseError is a constructor function that creates a new BaseError with
// only the message field set.
func NewBaseError(format string, a ...any) *BaseError {
Expand Down Expand Up @@ -180,11 +188,16 @@ func (e *BaseError) Details() map[string]string {
return e.details
}

// Details a Unique Code to identify the error
// Code returns a unique code to identify the error
func (e *BaseError) Code() ErrorCode {
return e.code
}

// Component is a method that returns the component field of BaseError.
func (e *BaseError) Component() string {
return e.component
}

// HTTPStatusCode is a method that returns the httpStatusCode field of BaseError.
// If no specific HTTP status code has been set, it returns 0.
// This method can be used to retrieve the HTTP status code associated with the error,
Expand Down
2 changes: 1 addition & 1 deletion pkg/publicapi/apimodels/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "github.com/bacalhau-project/bacalhau/pkg/models"
// IsAliveResponse is the response to the IsAlive request.
type IsAliveResponse struct {
BaseGetResponse `json:",omitempty,inline" yaml:",omitempty,inline"`
Status string
Status string `json:"Status"`
}

func (r *IsAliveResponse) IsReady() bool {
Expand Down
59 changes: 6 additions & 53 deletions pkg/publicapi/apimodels/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"net/http"
)

// apierror represents a standardized error response for the api.
// APIError represents a standardized error response for the api.
//
// it encapsulates:
// - an http status code
Expand All @@ -30,20 +30,20 @@ import (
type APIError struct {
// httpstatuscode is the http status code associated with this error.
// it should correspond to standard http status codes (e.g., 400, 404, 500).
HTTPStatusCode int `json:"-"`
HTTPStatusCode int `json:"Status"`

// message is a short, human-readable description of the error.
// it should be concise and provide a clear indication of what went wrong.
Message string `json:"message"`
Message string `json:"Message"`

// RequestID is the request ID of the request that caused the error.
RequestID string `json:"-"`
RequestID string `json:"RequestID"`

// Code is the error code of the error.
Code string `json:"code"`
Code string `json:"Code"`

// Component is the component that caused the error.
Component string `json:"component"`
Component string `json:"Component"`
}

// NewAPIError creates a new APIError with the given HTTP status code and message.
Expand All @@ -54,58 +54,11 @@ func NewAPIError(statusCode int, message string) *APIError {
}
}

// NewBadRequestError creates an APIError for Bad Request (400) errors.
func NewBadRequestError(message string) *APIError {
return NewAPIError(http.StatusBadRequest, message)
}

// NewUnauthorizedError creates an APIError for Unauthorized (401) errors.
func NewUnauthorizedError(message string) *APIError {
return NewAPIError(http.StatusUnauthorized, message)
}

// NewForbiddenError creates an APIError for Forbidden (403) errors.
func NewForbiddenError(message string) *APIError {
return NewAPIError(http.StatusForbidden, message)
}

// NewNotFoundError creates an APIError for Not Found (404) errors.
func NewNotFoundError(message string) *APIError {
return NewAPIError(http.StatusNotFound, message)
}

// NewConflictError creates an APIError for Conflict (409) errors.
func NewConflictError(message string) *APIError {
return NewAPIError(http.StatusConflict, message)
}

// NewInternalServerError creates an APIError for Internal Server Error (500) errors.
func NewInternalServerError(message string) *APIError {
return NewAPIError(http.StatusInternalServerError, message)
}

func NewJobNotFound(jobID string) *APIError {
return NewAPIError(http.StatusNotFound, fmt.Sprintf("job id %s not found", jobID))
}

// IsNotFound checks if the error is an APIError with a Not Found status.
func IsNotFound(err error) bool {
apiErr, ok := err.(*APIError)
return ok && apiErr.HTTPStatusCode == http.StatusNotFound
}

// IsBadRequest checks if the error is an APIError with a Bad Request status.
func IsBadRequest(err error) bool {
apiErr, ok := err.(*APIError)
return ok && apiErr.HTTPStatusCode == http.StatusBadRequest
}

// IsInternalServerError checks if the error is an APIError with an Internal Server Error status.
func IsInternalServerError(err error) bool {
apiErr, ok := err.(*APIError)
return ok && apiErr.HTTPStatusCode == http.StatusInternalServerError
}

// Error implements the error interface, allowing APIError to be used as a standard Go error.
func (e *APIError) Error() string {
return e.Message
Expand Down
2 changes: 1 addition & 1 deletion pkg/publicapi/endpoint/agent/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func NewEndpoint(params EndpointParams) *Endpoint {
// @ID agent/alive
// @Tags Ops
// @Produce text/plain
// @Success 200 {string} string "OK"
// @Success 200 {object} apimodels.IsAliveResponse
// @Router /api/v1/agent/alive [get]
func (e *Endpoint) alive(c echo.Context) error {
return c.JSON(http.StatusOK, &apimodels.IsAliveResponse{
Expand Down
54 changes: 31 additions & 23 deletions pkg/publicapi/middleware/error_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ package middleware
import (
"net/http"

"github.com/bacalhau-project/bacalhau/pkg/models"
"github.com/bacalhau-project/bacalhau/pkg/publicapi/apimodels"
"github.com/labstack/echo/v4"
"github.com/rs/zerolog/log"

"github.com/bacalhau-project/bacalhau/pkg/models"
"github.com/bacalhau-project/bacalhau/pkg/publicapi/apimodels"
)

func CustomHTTPErrorHandler(err error, c echo.Context) {

var code int
var message string
var errorCode string
var component string
var (
code int
message string
errorCode string
component string
)

switch e := err.(type) {

Expand All @@ -23,49 +25,55 @@ func CustomHTTPErrorHandler(err error, c echo.Context) {
code = e.HTTPStatusCode()
message = e.Error()
errorCode = string(e.Code())
component = e.Component()

case *echo.HTTPError:
// This is needed, in case any other middleware throws an error. In
// such a scenario we just use it as the error code and the message.
// One such example being when request body size is larger then the max
// size accepted
code = e.Code
message = e.Message.(string)
message, _ = e.Message.(string)
errorCode = string(models.InternalError)
component = "APIServer"
if c.Echo().Debug && e.Internal != nil {
message += ". " + e.Internal.Error()
}

default:
// In an ideal world this should never happen. We should always have are errors
// from server as APIError. If output is this generic string, one should evaluate
// and map it to APIError and send in appropriate message.= http.StatusInternalServerError
message = "internal server error"
code = c.Response().Status
code = http.StatusInternalServerError
message = "Internal server error"
errorCode = string(models.InternalError)
component = "Unknown"

if c.Echo().Debug {
message = err.Error()
message += ". " + err.Error()
}
}

requestID := c.Request().Header.Get(echo.HeaderXRequestID)

// Don't override the status code if it is already been set.
// This is something that is advised by ECHO framework.
if !c.Response().Committed {
apiError := apimodels.APIError{
HTTPStatusCode: code,
Message: message,
RequestID: c.Request().Header.Get(echo.HeaderXRequestID),
Code: errorCode,
Component: component,
}
var responseErr error
if c.Request().Method == http.MethodHead {
err = c.NoContent(code)
responseErr = c.NoContent(code)
} else {
err = c.JSON(code, apimodels.APIError{
HTTPStatusCode: code,
Message: message,
RequestID: requestID,
Code: errorCode,
Component: component,
})
responseErr = c.JSON(code, apiError)
}
if err != nil {
log.Info().Msg("unable to send json response while handling error.")
if responseErr != nil {
log.Error().Err(responseErr).
Str("original_error", err.Error()).
Msg("Failed to send error response")
}
}

Expand Down
Loading

0 comments on commit 01be858

Please sign in to comment.