Introduce APIError for Sync Calls from CLI to Orchestrator Node

cmd/cli/job/describe.go

@@ -2,6 +2,7 @@ package job
 
 import (
 	"cmp"
+	"errors"
 	"fmt"
 	"slices"
 	"time"
@@ -82,13 +83,14 @@ func NewDescribeCmd() *cobra.Command {
 func (o *DescribeOptions) run(cmd *cobra.Command, args []string, api client.API) error {
 	ctx := cmd.Context()
 	jobID := args[0]
+
 	response, err := api.Jobs().Get(ctx, &apimodels.GetJobRequest{
 		JobID:   jobID,
 		Include: "executions,history",
 	})
 
 	if err != nil {
-		return fmt.Errorf("could not get job %s: %w", jobID, err)
+		return errors.New(err.Error())
 	}
 
 	if o.OutputOpts.Format != "" {

cmd/cli/job/executions.go

@@ -1,6 +1,7 @@
 package job
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"time"
@@ -147,7 +148,7 @@ func (o *ExecutionOptions) run(cmd *cobra.Command, args []string, api client.API
 		},
 	})
 	if err != nil {
-		return err
+		return errors.New(err.Error())
 	}
 
 	if err = output.Output(cmd, executionColumns, o.OutputOptions, response.Items); err != nil {

cmd/cli/job/get.go

@@ -1,6 +1,7 @@
 package job
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -108,7 +109,7 @@ func get(cmd *cobra.Command, cmdArgs []string, api client.API, cfg types.Bacalha
 		jobID,
 		OG.DownloadSettings,
 	); err != nil {
-		return fmt.Errorf("downloading job: %w", err)
+		return errors.New(err.Error())
 	}
 
 	return nil

cmd/cli/job/history.go

@@ -1,6 +1,7 @@
 package job
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -160,7 +161,7 @@ func (o *HistoryOptions) run(cmd *cobra.Command, args []string, api client.API)
 		},
 	})
 	if err != nil {
-		return err
+		return errors.New(err.Error())
 	}
 
 	if err = output.Output(cmd, historyColumns, o.OutputOptions, response.Items); err != nil {

cmd/util/download.go

@@ -3,6 +3,7 @@ package util
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -33,7 +34,7 @@ func DownloadResultsHandler(
 		JobID: jobID,
 	})
 	if err != nil {
-		Fatal(cmd, fmt.Errorf("could not get results for job %s: %w", jobID, err), 1)
+		return errors.New(err.Error())
 	}
 
 	if len(response.Items) == 0 {

pkg/jobstore/boltdb/store.go

@@ -20,6 +20,7 @@ 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"
 )
@@ -176,7 +177,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, bacerrors.NewJobNotFound(jobID)
+		return job, apimodels.NewJobNotFound(jobID)
 	}
 
 	err = b.marshaller.Unmarshal(data, &job)
@@ -202,7 +203,7 @@ func (b *BoltJobStore) reifyJobID(tx *bolt.Tx, jobID string) (string, error) {
 
 		switch len(found) {
 		case 0:
-			return "", bacerrors.NewJobNotFound(jobID)
+			return "", apimodels.NewJobNotFound(jobID)
 		case 1:
 			return found[0], nil
 		default:
@@ -909,7 +910,7 @@ func (b *BoltJobStore) deleteJob(tx *bolt.Tx, jobID string) error {
 
 	job, err := b.getJob(tx, jobID)
 	if err != nil {
-		return bacerrors.NewJobNotFound(jobID)
+		apimodels.NewJobNotFound(jobID)
 	}
 
 	tx.OnCommit(func() {

pkg/publicapi/apimodels/error.go

@@ -0,0 +1,131 @@
+package apimodels
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+)
+
+// apierror represents a standardized error response for the api.
+//
+// it encapsulates:
+//   - an http status code
+//   - a human-readable error message
+//
+// purpose:
+//   - primarily used to send synchronous errors from the orchestrator endpoint
+//   - provides a structured json error response to http clients
+//
+// usage:
+//   - when the cli interacts with an orchestrator node via an http client:
+//     1. the http client receives this structured json error
+//     2. the human-readable message is output to stdout
+//     3. the http status code allows for programmatic handling of different error types
+//
+// benefits:
+//   - ensures consistent error formatting across the api
+//   - facilitates both user-friendly error messages and machine-readable error codes
+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:"code"`
+
+	// 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"`
+}
+
+// NewAPIError creates a new APIError with the given HTTP status code and message.
+func NewAPIError(statusCode int, message string) *APIError {
+	return &APIError{
+		HTTPStatusCode: statusCode,
+		Message:        message,
+	}
+}
+
+// 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
+}
+
+// Parse HTTP Resposne to APIError
+func FromHttpResponse(resp *http.Response) (*APIError, error) {
+
+	if resp == nil {
+		return nil, errors.New("response is nil, cannot be unmarsheld to APIError")
+	}
+
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("error reading response body: %w")
+	}
+
+	var apiErr APIError
+	err = json.Unmarshal(body, &apiErr)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing response body: %w", err)
+	}
+
+	// If the JSON didn't include a status code, use the HTTP Status
+	if apiErr.HTTPStatusCode == 0 {
+		apiErr.HTTPStatusCode = resp.StatusCode
+	}
+
+	return &apiErr, nil
+}

pkg/publicapi/client/v2/client.go

@@ -55,12 +55,25 @@ type httpClient struct {
 // and deserialize the response into a response object
 func (c *httpClient) Get(ctx context.Context, endpoint string, in apimodels.GetRequest, out apimodels.GetResponse) error {
 	r := in.ToHTTPRequest()
-	_, resp, err := requireOK(c.doRequest(ctx, http.MethodGet, endpoint, r)) //nolint:bodyclose // this is being closed
-	if err != nil && resp != nil && resp.StatusCode == http.StatusUnauthorized {
-		return apimodels.ErrInvalidToken
-	} else if err != nil {
-		return err
+
+	_, resp, err := c.doRequest(ctx, http.MethodGet, endpoint, r) //nolint:bodyclose // this is being closed
+
+	if resp.StatusCode == http.StatusUnauthorized {
+		return apimodels.NewUnauthorizedError("invalid token")
 	}
+
+	var apiError *apimodels.APIError
+	if resp.StatusCode != http.StatusOK {
+		apiError, err = apimodels.FromHttpResponse(resp)
+		if err != nil {
+			return err
+		}
+	}
+
+	if apiError != nil {
+		return apiError
+	}
+
 	defer resp.Body.Close()
 
 	if out != nil {

pkg/publicapi/client/v2/errors.go

@@ -165,13 +165,6 @@ func requireOK(d time.Duration, resp *http.Response, e error) (time.Duration, *h
 // response codes and validates that the received response code is among them
 func requireStatusIn(statuses ...int) doRequestWrapper {
 	return func(d time.Duration, resp *http.Response, e error) (time.Duration, *http.Response, error) {
-		if e != nil {
-			if resp != nil {
-				_ = resp.Body.Close()
-			}
-			return d, nil, e
-		}
-
 		for _, status := range statuses {
 			if resp.StatusCode == status {
 				return d, resp, nil

pkg/publicapi/endpoint/orchestrator/job.go

@@ -76,8 +76,10 @@ func (e *Endpoint) getJob(c echo.Context) error { //nolint: gocyclo
 	if err := c.Bind(&args); err != nil {
 		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
 	}
+	log.Info().Msg("UDIT SUCKS")
 	job, err := e.store.GetJob(ctx, jobID)
 	if err != nil {
+		log.Error().Err(err)
 		return err
 	}
 	response := apimodels.GetJobResponse{

pkg/publicapi/middleware/error_handler.go

@@ -0,0 +1,47 @@
+package middleware
+
+import (
+	"net/http"
+
+	"github.com/bacalhau-project/bacalhau/pkg/publicapi/apimodels"
+	"github.com/labstack/echo/v4"
+	"github.com/rs/zerolog/log"
+)
+
+func CustomHTTPErrorHandler(err error, c echo.Context) {
+
+	var code int
+	var message string
+
+	switch e := err.(type) {
+
+	case *apimodels.APIError:
+		// If it is already our custom APIError, use its code and message
+		code = e.HTTPStatusCode
+		message = e.Message
+
+	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"
+	}
+
+	// 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 {
+
+		if c.Request().Method == http.MethodHead {
+			err = c.NoContent(code)
+		} else {
+			err = c.JSON(code, apimodels.APIError{
+				HTTPStatusCode: code,
+				Message:        message,
+			})
+		}
+		if err != nil {
+			log.Info().Msg("unable to send json response while handling error.")
+		}
+	}
+
+}

pkg/publicapi/server.go

@@ -140,6 +140,10 @@ func NewAPIServer(params ServerParams) (*Server, error) {
 		middleware.VersionNotifyLogger(middlewareLogger, *serverVersion),
 	)
 
+	// Add custom http error handler. This is a centralized error handler for
+	// the server
+	server.Router.HTTPErrorHandler = middleware.CustomHTTPErrorHandler
+
 	var tlsConfig *tls.Config
 	if params.AutoCertDomain != "" {
 		log.Ctx(context.TODO()).Debug().Msgf("Setting up auto-cert for %s", params.AutoCertDomain)