refactor(handlers): use apierr.Respond for object upstream errors

This commit is contained in:
Noooste
2026-04-20 00:20:15 +02:00
parent 849840b808
commit adb430c2b2
2 changed files with 23 additions and 38 deletions
+11 -30
View File
@@ -9,6 +9,7 @@ import (
"strings" "strings"
"time" "time"
"Noooste/garage-ui/internal/apierr"
"Noooste/garage-ui/internal/models" "Noooste/garage-ui/internal/models"
"Noooste/garage-ui/internal/services" "Noooste/garage-ui/internal/services"
@@ -122,9 +123,7 @@ func (h *ObjectHandler) ListObjects(c fiber.Ctx) error {
// List objects in the bucket // List objects in the bucket
objects, err := h.s3Service.ListObjects(ctx, bucketName, prefix, maxKeys, continuationToken) objects, err := h.s3Service.ListObjects(ctx, bucketName, prefix, maxKeys, continuationToken)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeListFailed, "Failed to list objects: "+err.Error()),
)
} }
return c.JSON(models.SuccessResponse(objects)) return c.JSON(models.SuccessResponse(objects))
@@ -186,9 +185,7 @@ func (h *ObjectHandler) UploadObject(c fiber.Ctx) error {
// Upload to Garage // Upload to Garage
uploadResult, err := h.s3Service.UploadObject(ctx, bucketName, key, fileHandle, contentType) uploadResult, err := h.s3Service.UploadObject(ctx, bucketName, key, fileHandle, contentType)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeUploadFailed, "Failed to upload object: "+err.Error()),
)
} }
return c.Status(fiber.StatusCreated).JSON(models.SuccessResponse(uploadResult)) return c.Status(fiber.StatusCreated).JSON(models.SuccessResponse(uploadResult))
@@ -238,9 +235,7 @@ func (h *ObjectHandler) CreateDirectory(c fiber.Ctx) error {
result, err := h.s3Service.CreateDirectoryMarker(ctx, bucketName, key) result, err := h.s3Service.CreateDirectoryMarker(ctx, bucketName, key)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeUploadFailed, "Failed to create directory: "+err.Error()),
)
} }
return c.Status(fiber.StatusCreated).JSON(models.SuccessResponse(result)) return c.Status(fiber.StatusCreated).JSON(models.SuccessResponse(result))
@@ -281,9 +276,7 @@ func (h *ObjectHandler) GetObject(c fiber.Ctx) error {
// Get object from Garage // Get object from Garage
body, objectInfo, err := h.s3Service.GetObject(ctx, bucketName, key) body, objectInfo, err := h.s3Service.GetObject(ctx, bucketName, key)
if err != nil { if err != nil {
return c.Status(fiber.StatusNotFound).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeObjectNotFound, "Object not found: "+err.Error()),
)
} }
// The uploader controls Content-Type. Rewrite executable MIME types to // The uploader controls Content-Type. Rewrite executable MIME types to
@@ -345,9 +338,7 @@ func (h *ObjectHandler) DeleteObject(c fiber.Ctx) error {
// Check if object exists // Check if object exists
exists, err := h.s3Service.ObjectExists(ctx, bucketName, key) exists, err := h.s3Service.ObjectExists(ctx, bucketName, key)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeInternalError, "Failed to check object existence: "+err.Error()),
)
} }
if !exists { if !exists {
@@ -358,9 +349,7 @@ func (h *ObjectHandler) DeleteObject(c fiber.Ctx) error {
// Delete the object // Delete the object
if err := h.s3Service.DeleteObject(ctx, bucketName, key); err != nil { if err := h.s3Service.DeleteObject(ctx, bucketName, key); err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeDeleteFailed, "Failed to delete object: "+err.Error()),
)
} }
// Return success response // Return success response
@@ -407,9 +396,7 @@ func (h *ObjectHandler) GetObjectMetadata(c fiber.Ctx) error {
// Get object metadata // Get object metadata
metadata, err := h.s3Service.GetObjectMetadata(ctx, bucketName, key) metadata, err := h.s3Service.GetObjectMetadata(ctx, bucketName, key)
if err != nil { if err != nil {
return c.Status(fiber.StatusNotFound).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeObjectNotFound, "Object not found: "+err.Error()),
)
} }
return c.JSON(models.SuccessResponse(metadata)) return c.JSON(models.SuccessResponse(metadata))
@@ -467,9 +454,7 @@ func (h *ObjectHandler) GetPresignedURL(c fiber.Ctx) error {
// Check if object exists // Check if object exists
exists, err := h.s3Service.ObjectExists(ctx, bucketName, key) exists, err := h.s3Service.ObjectExists(ctx, bucketName, key)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeInternalError, "Failed to check object existence: "+err.Error()),
)
} }
if !exists { if !exists {
@@ -481,9 +466,7 @@ func (h *ObjectHandler) GetPresignedURL(c fiber.Ctx) error {
// Generate pre-signed URL // Generate pre-signed URL
url, err := h.s3Service.GetPresignedURL(ctx, bucketName, key, time.Duration(expiresIn)*time.Second) url, err := h.s3Service.GetPresignedURL(ctx, bucketName, key, time.Duration(expiresIn)*time.Second)
if err != nil { if err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeInternalError, "Failed to generate pre-signed URL: "+err.Error()),
)
} }
response := models.PresignedURLResponse{ response := models.PresignedURLResponse{
@@ -540,9 +523,7 @@ func (h *ObjectHandler) DeleteMultipleObjects(c fiber.Ctx) error {
// Delete multiple objects // Delete multiple objects
if err := h.s3Service.DeleteMultipleObjects(ctx, bucketName, req.Keys); err != nil { if err := h.s3Service.DeleteMultipleObjects(ctx, bucketName, req.Keys); err != nil {
return c.Status(fiber.StatusInternalServerError).JSON( return apierr.Respond(c, err)
models.ErrorResponse(models.ErrCodeDeleteFailed, "Failed to delete objects: "+err.Error()),
)
} }
response := models.ObjectDeleteMultipleResponse{ response := models.ObjectDeleteMultipleResponse{
+12 -8
View File
@@ -144,18 +144,20 @@ func TestGetObjectMetadata_Success(t *testing.T) {
} }
} }
func TestGetObjectMetadata_NotFound404(t *testing.T) { func TestGetObjectMetadata_ServiceError500(t *testing.T) {
app, s3 := newObjectsTestApp(t) app, s3 := newObjectsTestApp(t)
s3.GetObjectMetadataFn = func(_ context.Context, _, _ string) (*models.ObjectInfo, error) { s3.GetObjectMetadataFn = func(_ context.Context, _, _ string) (*models.ObjectInfo, error) {
return nil, errors.New("not found") return nil, errors.New("boom")
} }
resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/buckets/b1/objects/nope/metadata", nil)) resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/buckets/b1/objects/nope/metadata", nil))
if err != nil { if err != nil {
t.Fatalf("app.Test: %v", err) t.Fatalf("app.Test: %v", err)
} }
defer resp.Body.Close() defer resp.Body.Close()
if resp.StatusCode != http.StatusNotFound { // Generic errors map to 500/INTERNAL_ERROR; typed upstream NoSuchKey → 404
t.Fatalf("status = %d, want 404", resp.StatusCode) // is covered by Task 12 tests.
if resp.StatusCode != http.StatusInternalServerError {
t.Fatalf("status = %d, want 500", resp.StatusCode)
} }
} }
@@ -363,18 +365,20 @@ func TestGetObject_DownloadQuerySetsAttachment(t *testing.T) {
} }
} }
func TestGetObject_ServiceErrorReturns404(t *testing.T) { func TestGetObject_ServiceErrorReturns500(t *testing.T) {
app, s3 := newObjectsTestApp(t) app, s3 := newObjectsTestApp(t)
s3.GetObjectFn = func(_ context.Context, _, _ string) (io.ReadCloser, *models.ObjectInfo, error) { s3.GetObjectFn = func(_ context.Context, _, _ string) (io.ReadCloser, *models.ObjectInfo, error) {
return nil, nil, errors.New("not found") return nil, nil, errors.New("boom")
} }
resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/buckets/b1/objects/nope", nil)) resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/buckets/b1/objects/nope", nil))
if err != nil { if err != nil {
t.Fatalf("app.Test: %v", err) t.Fatalf("app.Test: %v", err)
} }
defer resp.Body.Close() defer resp.Body.Close()
if resp.StatusCode != http.StatusNotFound { // Generic errors map to 500/INTERNAL_ERROR; typed upstream NoSuchKey → 404
t.Fatalf("status = %d, want 404", resp.StatusCode) // is covered by Task 12 tests.
if resp.StatusCode != http.StatusInternalServerError {
t.Fatalf("status = %d, want 500", resp.StatusCode)
} }
} }