Improved error handling

Author: ewoutp

client/error.go

@@ -43,6 +43,7 @@ var (
 	InternalServerError = StatusError{StatusCode: http.StatusInternalServerError, message: "internal server error"}
 )
 
+// StatusError is an error with a given HTTP status code.
 type StatusError struct {
 	StatusCode int
 	message    string
@@ -75,6 +76,7 @@ func IsStatusErrorWithCode(err error, code int) bool {
 	return false
 }
 
+// ErrorResponse is the JSON structure returned in an API error.
 type ErrorResponse struct {
 	Error string
 }
@@ -99,6 +101,26 @@ func IsInternalServer(err error) bool {
 	return IsStatusErrorWithCode(err, http.StatusInternalServerError)
 }
 
+// NewServiceUnavailableError creates a service unavailable error with given message.
+func NewServiceUnavailableError(msg string) error {
+	return StatusError{StatusCode: http.StatusServiceUnavailable, message: msg}
+}
+
+// NewBadRequestError creates a bad request error with given message.
+func NewBadRequestError(msg string) error {
+	return StatusError{StatusCode: http.StatusBadRequest, message: msg}
+}
+
+// NewPreconditionFailedError creates a precondition failed error with given message.
+func NewPreconditionFailedError(msg string) error {
+	return StatusError{StatusCode: http.StatusPreconditionFailed, message: msg}
+}
+
+// NewInternalServerError creates a internal server error with given message.
+func NewInternalServerError(msg string) error {
+	return StatusError{StatusCode: http.StatusInternalServerError, message: msg}
+}
+
 // ParseResponseError returns an error from given response.
 // It tries to parse the body (if given body is nil, will be read from response)
 // for ErrorResponse.

service/server.go

@@ -592,6 +592,7 @@ func (s *httpServer) databaseAutoUpgradeHandler(w http.ResponseWriter, r *http.R
 
 	if !isRunning {
 		// We must have reached the running state before we can handle this kind of request
+		s.log.Debug().Msg("Received /database-auto-upgrade request while not in running phase")
 		writeError(w, http.StatusBadRequest, "Must be in running state to do upgrades")
 		return
 	}
@@ -625,6 +626,7 @@ func (s *httpServer) databaseAutoUpgradeHandler(w http.ResponseWriter, r *http.R
 				handleError(w, err)
 			} else {
 				if err := c.StartDatabaseUpgrade(ctx, force); err != nil {
+					s.log.Debug().Err(err).Msg("Forwarding StartDatabaseUpgrade failed")
 					handleError(w, err)
 				} else {
 					w.WriteHeader(http.StatusOK)
@@ -650,6 +652,7 @@ func (s *httpServer) databaseAutoUpgradeHandler(w http.ResponseWriter, r *http.R
 				handleError(w, err)
 			} else {
 				if err := c.RetryDatabaseUpgrade(ctx); err != nil {
+					s.log.Debug().Err(err).Msg("Forwarding RetryDatabaseUpgrade failed")
 					handleError(w, err)
 				} else {
 					w.WriteHeader(http.StatusOK)

service/service.go

@@ -788,7 +788,7 @@ func (s *Service) HandleHello(ownAddress, remoteAddress string, req *HelloReques
 			location := master.CreateStarterURL("/hello")
 			return ClusterConfig{}, maskAny(RedirectError{location})
 		} else {
-			return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "No master known"))
+			return ClusterConfig{}, maskAny(client.NewBadRequestError("No master known"))
 		}
 	}
 
@@ -830,7 +830,7 @@ func (s *Service) HandleHello(ownAddress, remoteAddress string, req *HelloReques
 		if slaveAddr == "" {
 			host, _, err := net.SplitHostPort(remoteAddress)
 			if err != nil {
-				return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "SlaveAddress must be set."))
+				return ClusterConfig{}, maskAny(client.NewBadRequestError("SlaveAddress must be set."))
 			}
 			slaveAddr = normalizeHostName(host)
 		} else {
@@ -840,21 +840,21 @@ func (s *Service) HandleHello(ownAddress, remoteAddress string, req *HelloReques
 
 		// Check request
 		if req.SlaveID == "" {
-			return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "SlaveID must be set."))
+			return ClusterConfig{}, maskAny(client.NewBadRequestError("SlaveID must be set."))
 		}
 
 		// Check datadir
 		if !s.allowSameDataDir {
 			for _, p := range s.myPeers.AllPeers {
 				if p.Address == slaveAddr && p.DataDir == req.DataDir && p.ID != req.SlaveID {
-					return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "Cannot use same directory as peer."))
+					return ClusterConfig{}, maskAny(client.NewBadRequestError("Cannot use same directory as peer."))
 				}
 			}
 		}
 
 		// Check IsSecure, cannot mix secure / non-secure
 		if req.IsSecure != s.IsSecure() {
-			return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "Cannot mix secure / non-secure peers."))
+			return ClusterConfig{}, maskAny(client.NewBadRequestError("Cannot mix secure / non-secure peers."))
 		}
 
 		// If slaveID already known, then return data right away.
@@ -869,7 +869,7 @@ func (s *Service) HandleHello(ownAddress, remoteAddress string, req *HelloReques
 					} else {
 						// Slave address may not change
 						if p.Address != slaveAddr {
-							return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "Cannot change slave address while using an existing ID."))
+							return ClusterConfig{}, maskAny(client.NewBadRequestError("Cannot change slave address while using an existing ID."))
 						}
 					}
 					s.myPeers.AllPeers[i].DataDir = req.DataDir
@@ -878,7 +878,7 @@ func (s *Service) HandleHello(ownAddress, remoteAddress string, req *HelloReques
 		} else {
 			// In single server mode, do not accept new slaves
 			if s.mode.IsSingleMode() {
-				return ClusterConfig{}, maskAny(errors.Wrap(client.BadRequestError, "In single server mode, slaves cannot be added."))
+				return ClusterConfig{}, maskAny(client.NewBadRequestError("In single server mode, slaves cannot be added."))
 			}
 			// Ok. We're now in cluster or resilient single mode.
 			// ID not yet found, add it

service/upgrade_manager.go

@@ -273,12 +273,12 @@ func (m *upgradeManager) StartDatabaseUpgrade(ctx context.Context, force bool) e
 	plan, err := m.readUpgradePlan(ctx)
 	if err != nil && !agency.IsKeyNotFound(err) {
 		// Failed to read upgrade plan
-		return errors.Wrapf(err, "Failed to read upgrade plan")
+		return errors.Wrap(err, "Failed to read upgrade plan")
 	}
 
 	// Check plan status
 	if !plan.IsReady() && !force {
-		return errors.Wrap(client.BadRequestError, "Current upgrade plan has not finished yet")
+		return maskAny(client.NewBadRequestError("Current upgrade plan has not finished yet"))
 	}
 
 	// Create upgrade plan
@@ -375,14 +375,14 @@ func (m *upgradeManager) RetryDatabaseUpgrade(ctx context.Context) error {
 
 	if !mode.HasAgency() {
 		// Without an agency there is not upgrade plan to retry
-		return errors.Wrap(client.BadRequestError, "Retry needs an agency")
+		return maskAny(client.NewBadRequestError("Retry needs an agency"))
 	}
 
 	// Retry upgrade with agency.
 	plan, err := m.readUpgradePlan(ctx)
 	if agency.IsKeyNotFound(err) {
 		// There is no upgrade plan
-		return errors.Wrap(client.BadRequestError, "There is no upgrade plan")
+		return maskAny(client.NewBadRequestError("There is no upgrade plan"))
 	}
 	if err != nil {
 		// Failed to read upgrade plan
@@ -391,7 +391,7 @@ func (m *upgradeManager) RetryDatabaseUpgrade(ctx context.Context) error {
 
 	// Check failure status
 	if !plan.IsFailed() {
-		return errors.Wrap(client.BadRequestError, "The upgrade plan has not failed")
+		return maskAny(client.NewBadRequestError("The upgrade plan has not failed"))
 	}
 
 	// Reset failures and write plan