Update error reporting middleware

router/error.go

@@ -14,45 +14,42 @@ import (
 )
 
 type RequestError struct {
-	Err     error
+	err     error
-	Uuid    string
+	uuid    string
-	Message string
+	message string
 	server  *server.Server
 }
 
 // Generates a new tracked error, which simply tracks the specific error that
 // is being passed in, and also assigned a UUID to the error so that it can be
 // cross referenced in the logs.
-func TrackedError(err error) *RequestError {
+func NewTrackedError(err error) *RequestError {
 	return &RequestError{
-		Err:     err,
+		err:  err,
-		Uuid:    uuid.Must(uuid.NewRandom()).String(),
+		uuid: uuid.Must(uuid.NewRandom()).String(),
-		Message: "",
 	}
 }
 
-// Same as TrackedError, except this will also attach the server instance that
+// Same as NewTrackedError, except this will also attach the server instance that
 // generated this server for the purposes of logging.
-func TrackedServerError(err error, s *server.Server) *RequestError {
+func NewServerError(err error, s *server.Server) *RequestError {
 	return &RequestError{
-		Err:     err,
+		err:    err,
-		Uuid:    uuid.Must(uuid.NewRandom()).String(),
+		uuid:   uuid.Must(uuid.NewRandom()).String(),
-		Message: "",
 		server: s,
 	}
 }
 
 func (e *RequestError) logger() *log.Entry {
 	if e.server != nil {
-		return e.server.Log().WithField("error_id", e.Uuid)
+		return e.server.Log().WithField("error_id", e.uuid)
 	}
-
+	return log.WithField("error_id", e.uuid)
-	return log.WithField("error_id", e.Uuid)
 }
 
 // Sets the output message to display to the user in the error.
 func (e *RequestError) SetMessage(msg string) *RequestError {
-	e.Message = msg
+	e.message = msg
 	return e
 }
 
@@ -60,35 +57,43 @@ func (e *RequestError) SetMessage(msg string) *RequestError {
 // will also include the error UUID in the output so that the user can report that
 // and link the response to a specific error in the logs.
 func (e *RequestError) AbortWithStatus(status int, c *gin.Context) {
+	// In instances where the status has already been set just use that existing status
+	// since we cannot change it at this point, and trying to do so will emit a gin warning
+	// into the program output.
+	if c.Writer.Status() != 200 {
+		status = c.Writer.Status()
+	}
+
 	// If this error is because the resource does not exist, we likely do not need to log
 	// the error anywhere, just return a 404 and move on with our lives.
-	if errors.Is(e.Err, os.ErrNotExist) {
+	if errors.Is(e.err, os.ErrNotExist) {
-		e.logger().WithField("error", e.Err).Debug("encountered os.IsNotExist error while handling request")
+		e.logger().WithField("error", e.err).Debug("encountered os.IsNotExist error while handling request")
-
 		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
 			"error": "The requested resource was not found on the system.",
 		})
 		return
 	}
 
+	// If this is a Filesystem error just return it without all of the tracking code nonsense
+	// since we don't need to be logging it into the logs or anything, its just a normal error
+	// that the user can solve on their end.
+	if st, msg := e.getAsFilesystemError(); st != 0 {
+		c.AbortWithStatusJSON(st, gin.H{"error": msg})
+		return
+	}
+
 	// Otherwise, log the error to zap, and then report the error back to the user.
 	if status >= 500 {
-		e.logger().WithField("error", e.Err).Error("encountered HTTP/500 error while handling request")
+		e.logger().WithField("error", e.err).Error("unexpected error while handling HTTP request")
-
-		c.Error(e)
 	} else {
-		e.logger().WithField("error", e.Err).Debug("encountered non-HTTP/500 error while handling request")
+		e.logger().WithField("error", e.err).Debug("non-server error encountered while handling HTTP request")
 	}
 
-	msg := "An unexpected error was encountered while processing this request."
+	if e.message == "" {
-	if e.Message != "" {
+		e.message = "An unexpected error was encountered while processing this request."
-		msg = e.Message
 	}
 
-	c.AbortWithStatusJSON(status, gin.H{
+	c.AbortWithStatusJSON(status, gin.H{"error": e.message, "error_id": e.uuid})
-		"error":    msg,
-		"error_id": e.Uuid,
-	})
 }
 
 // Helper function to just abort with an internal server error. This is generally the response
@@ -97,41 +102,30 @@ func (e *RequestError) Abort(c *gin.Context) {
 	e.AbortWithStatus(http.StatusInternalServerError, c)
 }
 
+// Looks at the given RequestError and determines if it is a specific filesystem error that
+// we can process and return differently for the user.
+func (e *RequestError) getAsFilesystemError() (int, string) {
+	if errors.Is(e.err, os.ErrNotExist) || filesystem.IsErrorCode(e.err, filesystem.ErrCodePathResolution) {
+		return http.StatusNotFound, "The requested resource was not found on the system."
+	}
+	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeDiskSpace) {
+		return http.StatusConflict, "There is not enough disk space available to perform that action."
+	}
+	if strings.HasSuffix(e.err.Error(), "file name too long") {
+		return http.StatusBadRequest, "Cannot perform that action: file name is too long."
+	}
+	if e, ok := e.err.(*os.SyscallError); ok && e.Syscall == "readdirent" {
+		return http.StatusNotFound, "The requested directory does not exist."
+	}
+	return 0, ""
+}
+
 // Handle specific filesystem errors for a server.
 func (e *RequestError) AbortFilesystemError(c *gin.Context) {
-	if errors.Is(e.Err, os.ErrNotExist) || filesystem.IsErrorCode(e.Err, filesystem.ErrCodePathResolution) {
-		if filesystem.IsErrorCode(e.Err, filesystem.ErrCodePathResolution) {
-			e.logger().Warn(e.Err.Error())
-		}
-
-		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"error": "The requested resource was not found."})
-		return
-	}
-
-	if filesystem.IsErrorCode(e.Err, filesystem.ErrCodeDiskSpace) {
-		c.AbortWithStatusJSON(http.StatusConflict, gin.H{"error": "There is not enough disk space available to perform that action."})
-		return
-	}
-
-	if strings.HasSuffix(e.Err.Error(), "file name too long") {
-		c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "File name is too long."})
-		return
-	}
-
-	if e, ok := e.Err.(*os.SyscallError); ok && e.Syscall == "readdirent" {
-		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"error": "The requested directory does not exist."})
-		return
-	}
-
-	if strings.HasSuffix(e.Err.Error(), "file name too long") {
-		c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "Cannot perform that action: file name is too long."})
-		return
-	}
-
 	e.Abort(c)
 }
 
 // Format the error to a string and include the UUID.
 func (e *RequestError) Error() string {
-	return fmt.Sprintf("%v (uuid: %s)", e.Err, e.Uuid)
+	return fmt.Sprintf("%v (uuid: %s)", e.err, e.uuid)
 }

router/middleware.go

@@ -22,23 +22,15 @@ func (m *Middleware) ErrorHandler() gin.HandlerFunc {
 		if err == nil {
 			return
 		}
-
+		tracked := NewTrackedError(err)
-		tracked := TrackedError(err)
 		// If there is a server in the context for this request pull it out so that we can
 		// track the error specifically for that server.
 		if s, ok := c.Get("server"); ok {
-			tracked = TrackedServerError(err, s.(*server.Server))
+			tracked = NewServerError(err, s.(*server.Server))
 		}
-
+		// This error occurs if you submit invalid JSON data to an endpoint.
-		// Sometimes requests have already modifed the status by the time this handler is
-		// called. In those cases, try to attach the error message but don't try to change
-		// the response status since it has already been set.
-		if c.Writer.Status() != 200 {
 		if err.Error() == io.EOF.Error() {
 			c.JSON(c.Writer.Status(), gin.H{"error": "A JSON formatted body is required for this endpoint."})
-			} else {
-				tracked.AbortWithStatus(c.Writer.Status(), c)
-			}
 			return
 		}
 		tracked.Abort(c)

router/router_download.go

@@ -15,7 +15,7 @@ import (
 func getDownloadBackup(c *gin.Context) {
 	token := tokens.BackupPayload{}
 	if err := tokens.ParseToken([]byte(c.Query("token")), &token); err != nil {
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 
@@ -36,13 +36,13 @@ func getDownloadBackup(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
 	f, err := os.Open(b.Path())
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 	defer f.Close()
@@ -58,7 +58,7 @@ func getDownloadBackup(c *gin.Context) {
 func getDownloadFile(c *gin.Context) {
 	token := tokens.FilePayload{}
 	if err := tokens.ParseToken([]byte(c.Query("token")), &token); err != nil {
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 
@@ -75,7 +75,7 @@ func getDownloadFile(c *gin.Context) {
 	// If there is an error or we're somehow trying to download a directory, just
 	// respond with the appropriate error.
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	} else if st.IsDir() {
 		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
@@ -86,7 +86,7 @@ func getDownloadFile(c *gin.Context) {
 
 	f, err := os.Open(p)
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 

router/router_server.go

@@ -41,7 +41,7 @@ func getServerLogs(c *gin.Context) {
 
 	out, err := s.ReadLogfile(l)
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -110,7 +110,7 @@ func postServerCommands(c *gin.Context) {
 	s := GetServer(c.Param("server"))
 
 	if running, err := s.Environment.IsRunning(); err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	} else if !running {
 		c.AbortWithStatusJSON(http.StatusBadGateway, gin.H{
@@ -144,7 +144,7 @@ func patchServer(c *gin.Context) {
 	buf.ReadFrom(c.Request.Body)
 
 	if err := s.UpdateDataStructure(buf.Bytes()); err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -214,7 +214,7 @@ func deleteServer(c *gin.Context) {
 	// forcibly terminate it before removing the container, so we do not need to handle
 	// that here.
 	if err := s.Environment.Destroy(); err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 	}
 
 	// Once the environment is terminated, remove the server files from the system. This is

router/router_server_backup.go

@@ -34,7 +34,7 @@ func postServerBackup(c *gin.Context) {
 	}
 
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -63,7 +63,7 @@ func deleteServerBackup(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -72,7 +72,7 @@ func deleteServerBackup(c *gin.Context) {
 		// the backup previously and it is now missing when we go to delete, just treat it as having
 		// been successful, rather than returning a 404.
 		if !errors.Is(err, os.ErrNotExist) {
-			TrackedServerError(err, s).Abort(c)
+			NewServerError(err, s).Abort(c)
 			return
 		}
 	}

router/router_server_files.go

@@ -25,7 +25,7 @@ func getServerFileContents(c *gin.Context) {
 	p := "/" + strings.TrimLeft(c.Query("file"), "/")
 	st, err := s.Filesystem().Stat(p)
 	if err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -49,7 +49,7 @@ func getServerFileContents(c *gin.Context) {
 	// happen since we're doing so much before this point that would normally throw an error if there
 	// was a problem with the file.
 	if err := s.Filesystem().Readfile(p, c.Writer); err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 }
@@ -60,7 +60,7 @@ func getServerListDirectory(c *gin.Context) {
 
 	stats, err := s.Filesystem().ListDirectory(c.Query("directory"))
 	if err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -127,7 +127,7 @@ func putServerRenameFiles(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -147,7 +147,7 @@ func postServerCopyFile(c *gin.Context) {
 	}
 
 	if err := s.Filesystem().Copy(data.Location); err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -192,7 +192,7 @@ func postServerDeleteFiles(c *gin.Context) {
 	}
 
 	if err := g.Wait(); err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -205,7 +205,7 @@ func postServerWriteFile(c *gin.Context) {
 
 	f, err := url.QueryUnescape(c.Query("file"))
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 	f = "/" + strings.TrimLeft(f, "/")
@@ -218,7 +218,7 @@ func postServerWriteFile(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -244,13 +244,13 @@ func postServerDownloadRemoteFile(c *gin.Context) {
 			})
 			return
 		}
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
 	resp, err := http.Get(u.String())
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 	defer resp.Body.Close()
@@ -263,7 +263,7 @@ func postServerDownloadRemoteFile(c *gin.Context) {
 			})
 			return
 		}
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -291,7 +291,7 @@ func postServerCreateDirectory(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -326,7 +326,7 @@ func postServerCompressFiles(c *gin.Context) {
 
 	f, err := s.Filesystem().CompressFiles(data.RootPath, data.Files)
 	if err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -359,7 +359,7 @@ func postServerDecompressFiles(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -390,7 +390,7 @@ func postServerDecompressFiles(c *gin.Context) {
 			return
 		}
 
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -452,7 +452,7 @@ func postServerChmodFile(c *gin.Context) {
 	}
 
 	if err := g.Wait(); err != nil {
-		TrackedServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).AbortFilesystemError(c)
 		return
 	}
 
@@ -462,7 +462,7 @@ func postServerChmodFile(c *gin.Context) {
 func postServerUploadFiles(c *gin.Context) {
 	token := tokens.UploadPayload{}
 	if err := tokens.ParseToken([]byte(c.Query("token")), &token); err != nil {
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 
@@ -500,14 +500,14 @@ func postServerUploadFiles(c *gin.Context) {
 	for _, header := range headers {
 		p, err := s.Filesystem().SafePath(filepath.Join(directory, header.Filename))
 		if err != nil {
-			TrackedServerError(err, s).AbortFilesystemError(c)
+			NewServerError(err, s).AbortFilesystemError(c)
 			return
 		}
 
 		// We run this in a different method so I can use defer without any of
 		// the consequences caused by calling it in a loop.
 		if err := handleFileUpload(p, s, header); err != nil {
-			TrackedServerError(err, s).AbortFilesystemError(c)
+			NewServerError(err, s).AbortFilesystemError(c)
 			return
 		}
 	}

router/router_server_ws.go

@@ -14,7 +14,7 @@ func getServerWebsocket(c *gin.Context) {
 	s := GetServer(c.Param("server"))
 	handler, err := websocket.GetHandler(s, c.Writer, c.Request)
 	if err != nil {
-		TrackedServerError(err, s).Abort(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 	defer handler.Connection.Close()

router/router_system.go

@@ -16,7 +16,7 @@ import (
 func getSystemInformation(c *gin.Context) {
 	i, err := system.GetSystemInformation()
 	if err != nil {
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 
 		return
 	}
@@ -45,7 +45,7 @@ func postCreateServer(c *gin.Context) {
 			return
 		}
 
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 
@@ -99,7 +99,7 @@ func postUpdateConfiguration(c *gin.Context) {
 		// before this code was run.
 		config.Set(&ccopy)
 
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 

router/router_transfer.go

@@ -37,7 +37,7 @@ func getServerArchive(c *gin.Context) {
 
 	token := tokens.TransferPayload{}
 	if err := tokens.ParseToken([]byte(auth[1]), &token); err != nil {
-		TrackedError(err).Abort(c)
+		NewTrackedError(err).Abort(c)
 		return
 	}
 
@@ -53,7 +53,7 @@ func getServerArchive(c *gin.Context) {
 	st, err := s.Archiver.Stat()
 	if err != nil {
 		if !errors.Is(err, os.ErrNotExist) {
-			TrackedServerError(err, s).SetMessage("failed to stat archive").Abort(c)
+			NewServerError(err, s).SetMessage("failed to stat archive").Abort(c)
 			return
 		}
 
@@ -63,13 +63,13 @@ func getServerArchive(c *gin.Context) {
 
 	checksum, err := s.Archiver.Checksum()
 	if err != nil {
-		TrackedServerError(err, s).SetMessage("failed to calculate checksum").Abort(c)
+		NewServerError(err, s).SetMessage("failed to calculate checksum").Abort(c)
 		return
 	}
 
 	file, err := os.Open(s.Archiver.Path())
 	if err != nil {
-		tserr := TrackedServerError(err, s)
+		tserr := NewServerError(err, s)
 		if !os.IsNotExist(err) {
 			tserr.SetMessage("failed to open archive for reading")
 		} else {