From 99e55d4d12bbd822b739d4fa32f5a636280ccc3f Mon Sep 17 00:00:00 2001 From: celogeek Date: Sat, 7 May 2022 15:38:44 +0200 Subject: [PATCH] improve errors management --- internal/photos/api/account.go | 22 +++++++-------- internal/photos/api/check_body.go | 2 +- internal/photos/api/errors.go | 24 +++++++++-------- internal/photos/api/file.go | 45 +++++++++++++++---------------- internal/photos/api/main.go | 3 ++- internal/photos/api/recovery.go | 6 ++++- internal/photos/api/session.go | 10 +++---- internal/photos/api/upload.go | 34 ++++++++++++++--------- 8 files changed, 80 insertions(+), 66 deletions(-) diff --git a/internal/photos/api/account.go b/internal/photos/api/account.go index 6054640..bfa6a8c 100644 --- a/internal/photos/api/account.go +++ b/internal/photos/api/account.go @@ -59,26 +59,25 @@ type SignupOrLoginRequest struct { func (s *Service) Signup(c *gin.Context) { var account *SignupOrLoginRequest - if err := c.ShouldBindJSON(&account); err != nil { - s.Error(c, http.StatusBadRequest, err) + if c.BindJSON(&account) != nil { return } if err := validator.Validate(account); err != nil { - s.Error(c, http.StatusExpectationFailed, err) + c.AbortWithError(http.StatusExpectationFailed, err) return } var accountExists int64 if err := s.DB.Model(&Account{}).Where("login = ?", account.Login).Count(&accountExists).Error; err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } if accountExists > 0 { - s.Error(c, http.StatusConflict, ErrAccountExists) + c.AbortWithError(http.StatusConflict, ErrAccountExists) return } if err := s.DB.Create(NewAccount(account.Login, account.Password)).Error; err != nil { - s.Error(c, http.StatusConflict, err) + c.AbortWithError(http.StatusConflict, err) return } @@ -90,17 +89,16 @@ func (s *Service) Signup(c *gin.Context) { func (s *Service) Login(c *gin.Context) { var account *SignupOrLoginRequest - if err := c.ShouldBindJSON(&account); err != nil { - s.Error(c, http.StatusBadRequest, err) + if c.BindJSON(&account) != nil { return } session, err := NewSession(s.DB, account.Login, account.Password) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { - s.Error(c, http.StatusNotFound, ErrAccountAuth) + c.AbortWithError(http.StatusNotFound, ErrAccountAuth) } else { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) } return } @@ -114,11 +112,11 @@ func (s *Service) Login(c *gin.Context) { func (s *Service) Logout(c *gin.Context) { res := s.DB.Where("token = ?", c.GetString("token")).Delete(&Session{}) if res.Error != nil { - s.Error(c, http.StatusInternalServerError, res.Error) + c.AbortWithError(http.StatusInternalServerError, res.Error) return } if res.RowsAffected == 0 { - s.Error(c, http.StatusNotFound, ErrSessionNotFound) + c.AbortWithError(http.StatusNotFound, ErrSessionNotFound) return } c.JSON(http.StatusOK, gin.H{ diff --git a/internal/photos/api/check_body.go b/internal/photos/api/check_body.go index 4a63fd5..f28ce80 100644 --- a/internal/photos/api/check_body.go +++ b/internal/photos/api/check_body.go @@ -13,7 +13,7 @@ var ( func (s *Service) RequireBody(c *gin.Context) { if c.Request.Method == "POST" && c.Request.ContentLength == 0 { - s.Error(c, http.StatusBadRequest, ErrReqMissingBody) + c.AbortWithError(http.StatusBadRequest, ErrReqMissingBody) return } } diff --git a/internal/photos/api/errors.go b/internal/photos/api/errors.go index 674f3d5..64e3c6d 100644 --- a/internal/photos/api/errors.go +++ b/internal/photos/api/errors.go @@ -4,16 +4,18 @@ import ( "github.com/gin-gonic/gin" ) -func (s *Service) Error(c *gin.Context, code int, err error) { - var status string - if code >= 200 && code < 400 { - status = "success" - } else { - status = "failed" - } +var ( + StatusSuccess = "success" + StatusFailed = "failed" +) - c.AbortWithStatusJSON(code, gin.H{ - "status": status, - "error": err.Error(), - }) +func (s *Service) HandleError(c *gin.Context) { + c.Next() + + if err := c.Errors.Last(); err != nil { + c.JSON(-1, gin.H{ + "status": StatusFailed, + "error": err.Err.Error(), + }) + } } diff --git a/internal/photos/api/file.go b/internal/photos/api/file.go index 0db0bc8..c7fa392 100644 --- a/internal/photos/api/file.go +++ b/internal/photos/api/file.go @@ -63,28 +63,27 @@ type FileRequest struct { func (s *Service) FileCreate(c *gin.Context) { file := &FileRequest{} - if err := c.ShouldBindJSON(file); err != nil { - s.Error(c, http.StatusInternalServerError, err) + if c.BindJSON(file) != nil { return } if len(file.Name) < 1 { - s.Error(c, http.StatusBadRequest, ErrStoreMissingName) + c.AbortWithError(http.StatusBadRequest, ErrStoreMissingName) return } if len(file.Checksum) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } if len(file.Chunks) == 0 { - s.Error(c, http.StatusBadRequest, ErrStoreMissingChunks) + c.AbortWithError(http.StatusBadRequest, ErrStoreMissingChunks) return } for _, chunk := range file.Chunks { if len(chunk) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } } @@ -92,15 +91,15 @@ func (s *Service) FileCreate(c *gin.Context) { r, rs, err := s.Store.Combine(file.Chunks) if err != nil { if strings.HasPrefix(err.Error(), "chunk") && strings.HasSuffix(err.Error(), "doesn't exists") { - s.Error(c, http.StatusBadRequest, err) + c.AbortWithError(http.StatusBadRequest, err) } else { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) } return } if r != file.Checksum { - s.Error(c, http.StatusExpectationFailed, ErrStoreMismatchChecksum) + c.AbortWithError(http.StatusExpectationFailed, ErrStoreMismatchChecksum) return } @@ -137,7 +136,7 @@ func (s *Service) FileCreate(c *gin.Context) { } if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } @@ -151,7 +150,7 @@ func (s *Service) FileCreate(c *gin.Context) { func (s *Service) FileCreateChunk(c *gin.Context) { if c.Request.ContentLength > CHUNK_SIZE { - s.Error(c, http.StatusBadRequest, ErrStoreBadChunkSize) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChunkSize) return } @@ -169,7 +168,7 @@ func (s *Service) FileCreateChunk(c *gin.Context) { "checksum": chunk.Sum, }) } else { - s.Error(c, http.StatusBadRequest, err) + c.AbortWithError(http.StatusBadRequest, err) } return } @@ -183,7 +182,7 @@ func (s *Service) FileCreateChunk(c *gin.Context) { func (s *Service) FileChunkExists(c *gin.Context) { checksum := c.Param("checksum") if len(checksum) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } @@ -197,13 +196,13 @@ func (s *Service) FileChunkExists(c *gin.Context) { func (s *Service) FileExists(c *gin.Context) { checksum := c.Param("checksum") if len(checksum) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } var fileExists int64 if err := s.DB.Model(&File{}).Where("checksum = ?", checksum).Count(&fileExists).Error; err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } @@ -217,7 +216,7 @@ func (s *Service) FileExists(c *gin.Context) { func (s *Service) FileGet(c *gin.Context) { checksum := c.Param("checksum") if len(checksum) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } if checksum == c.GetHeader("If-None-Match") { @@ -227,7 +226,7 @@ func (s *Service) FileGet(c *gin.Context) { f := &File{} if err := s.DB.Debug().Preload("Chunks").Where("checksum = ?", checksum).First(&f).Error; err != nil { - s.Error(c, http.StatusBadRequest, err) + c.AbortWithError(http.StatusBadRequest, err) return } @@ -238,7 +237,7 @@ func (s *Service) FileGet(c *gin.Context) { reader, err := s.Store.NewStoreReader(chunks) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } defer reader.Close() @@ -258,13 +257,13 @@ func (s *Service) FileGet(c *gin.Context) { func (s *Service) FileAnalyze(c *gin.Context) { checksum := c.Param("checksum") if len(checksum) != 40 { - s.Error(c, http.StatusBadRequest, ErrStoreBadChecksum) + c.AbortWithError(http.StatusBadRequest, ErrStoreBadChecksum) return } f := &File{} if err := s.DB.Debug().Preload("Chunks").Where("checksum = ?", checksum).First(&f).Error; err != nil { - s.Error(c, http.StatusBadRequest, err) + c.AbortWithError(http.StatusBadRequest, err) return } @@ -275,19 +274,19 @@ func (s *Service) FileAnalyze(c *gin.Context) { reader, err := s.Store.NewStoreReader(chunks) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } defer reader.Close() rawExif, err := exif.SearchAndExtractExifWithReader(reader) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } entries, _, err := exif.GetFlatExifDataUniversalSearch(rawExif, nil, true) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } diff --git a/internal/photos/api/main.go b/internal/photos/api/main.go index 1dac4be..9ac10e6 100644 --- a/internal/photos/api/main.go +++ b/internal/photos/api/main.go @@ -50,6 +50,7 @@ func (s *Service) SetupRoutes() { s.Gin.Use( gin.Logger(), s.Recovery, + s.HandleError, s.RequireBody, ) @@ -59,7 +60,7 @@ func (s *Service) SetupRoutes() { s.UploadInit() s.Gin.NoRoute(func(c *gin.Context) { - s.Error(c, http.StatusNotFound, ErrReqNotFound) + c.AbortWithError(http.StatusNotFound, ErrReqNotFound) }) } diff --git a/internal/photos/api/recovery.go b/internal/photos/api/recovery.go index db2ce6d..99f3382 100644 --- a/internal/photos/api/recovery.go +++ b/internal/photos/api/recovery.go @@ -17,7 +17,11 @@ func (s *Service) Recovery(c *gin.Context) { defer func() { if err := recover(); err != nil { s.LogErr.Print("PANIC", err) - s.Error(c, http.StatusInternalServerError, ErrUnexpected) + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{ + "status": StatusFailed, + "error": ErrUnexpected.Error(), + "details": err, + }) } }() c.Next() diff --git a/internal/photos/api/session.go b/internal/photos/api/session.go index 53ff28e..79b1be2 100644 --- a/internal/photos/api/session.go +++ b/internal/photos/api/session.go @@ -61,14 +61,14 @@ func (s *Service) RequireAuthToken(c *gin.Context) { if tokenAuth != "" { if !strings.HasPrefix(tokenAuth, "Private ") { - s.Error(c, http.StatusForbidden, ErrTokenMissing) + c.AbortWithError(http.StatusForbidden, ErrTokenMissing) } else { c.Set("token", tokenAuth[8:]) } } else if tokenCookie != "" { c.Set("token", tokenCookie) } else { - s.Error(c, http.StatusForbidden, ErrTokenMissing) + c.AbortWithError(http.StatusForbidden, ErrTokenMissing) } } @@ -81,14 +81,14 @@ func (s *Service) RequireSession(c *gin.Context) { sess := &Session{} if err := s.DB.Preload("Account").Where("token = ?", c.GetString("token")).First(sess).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { - s.Error(c, http.StatusForbidden, ErrSessionNotFound) + c.AbortWithError(http.StatusForbidden, ErrSessionNotFound) } else { - s.Error(c, http.StatusForbidden, err) + c.AbortWithError(http.StatusForbidden, err) } return } if sess.Account == nil { - s.Error(c, http.StatusInternalServerError, ErrSessionInvalid) + c.AbortWithError(http.StatusInternalServerError, ErrSessionInvalid) return } s.DB.Select("updated_at").Save(sess) diff --git a/internal/photos/api/upload.go b/internal/photos/api/upload.go index 5880efc..aaa6353 100644 --- a/internal/photos/api/upload.go +++ b/internal/photos/api/upload.go @@ -13,15 +13,19 @@ import ( "github.com/google/uuid" ) +var ( + ErrUploadNotExists = errors.New("upload id doesn't exists") +) + func (s *Service) UploadCreate(c *gin.Context) { sha, err := uuid.NewRandom() if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } if err := s.Storage.Create(StorageTmp, sha.String()); err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } @@ -35,16 +39,23 @@ type UploadPartRequest struct { UploadId string `uri:"upload_id" binding:"required,uuid"` Part uint `uri:"part" binding:"required"` } +type UploadPartHeaders struct { + PartSha256 string `header:"x-part-sha256" binding:"required,lowercase,alphanum,len=64"` +} func (s *Service) UploadPart(c *gin.Context) { var uploadPart UploadPartRequest - if err := c.ShouldBindUri(&uploadPart); err != nil { - s.Error(c, http.StatusBadRequest, err) + if c.BindUri(&uploadPart) != nil { + return + } + + var uploadPartHeaders UploadPartHeaders + if c.BindHeader(&uploadPartHeaders) != nil { return } if !s.Storage.Exists(StorageTmp, uploadPart.UploadId) { - s.Error(c, http.StatusNotFound, errors.New("upload id doesn't exists")) + c.AbortWithError(http.StatusNotFound, ErrUploadNotExists) return } @@ -52,7 +63,7 @@ func (s *Service) UploadPart(c *gin.Context) { s.Storage.Join(StorageTmp, uploadPart.UploadId, fmt.Sprint(uploadPart.Part)), ) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } @@ -63,7 +74,7 @@ func (s *Service) UploadPart(c *gin.Context) { t := io.TeeReader(c.Request.Body, sha) w, err := io.Copy(f, t) if err != nil { - s.Error(c, http.StatusInternalServerError, err) + c.AbortWithError(http.StatusInternalServerError, err) return } @@ -80,7 +91,7 @@ func (s *Service) UploadCancel(c *gin.Context) { upload_id := c.Param("upload_id") if err := s.Storage.Delete(StorageTmp, upload_id); err != nil { - s.Error(c, http.StatusNotFound, errors.New("upload id doesn't exists")) + c.AbortWithError(http.StatusNotFound, ErrUploadNotExists) return } @@ -90,19 +101,18 @@ func (s *Service) UploadCancel(c *gin.Context) { } type UploadCompleteRequest struct { - SHA256 string `json:"sha256" binding:"required,lowercase,alphanum,len=64"` + Sha256 string `json:"sha256" binding:"required,lowercase,alphanum,len=64"` Name string `json:"name" binding:"required"` Parts uint `json:"parts" binding:"required"` } func (s *Service) UploadComplete(c *gin.Context) { var uploadCompleteRequest UploadCompleteRequest - if err := c.ShouldBindJSON(&uploadCompleteRequest); err != nil { - s.Error(c, http.StatusBadRequest, err) + if c.BindJSON(&uploadCompleteRequest) != nil { return } c.JSON(http.StatusOK, gin.H{ - "sha256": uploadCompleteRequest.SHA256, + "sha256": uploadCompleteRequest.Sha256, "parts": uploadCompleteRequest.Parts, "name": uploadCompleteRequest.Name, })