Add structured logging and centralized error handling

- Initialize slog in the serve command with terminal/file support
- Introduce `AppError` with HTTP status for unified service-layer errors
- Replace ad-hoc `api.Error` calls with `api.RespondError`
- Wrap internal errors with `model.NewInternalError` and add reference
  IDs
- Add `RequestID` middleware and switch router from `gin.Default` to
  `gin.New`
This commit is contained in:
2026-07-04 16:24:22 +08:00
parent a78d43b166
commit 1dfccf513a
16 changed files with 281 additions and 134 deletions
+30 -27
View File
@@ -3,7 +3,7 @@ package service
import (
"context"
"errors"
"fmt"
"net/http"
"strings"
"time"
@@ -59,12 +59,12 @@ func NewAuthService(
// Register creates a new user account.
func (s *AuthService) Register(ctx context.Context, username, email, password string) (*model.User, error) {
if username == "" || email == "" || password == "" {
return nil, fmt.Errorf("username, email, and password are required")
return nil, &model.AppError{Status: http.StatusBadRequest, Message: "username, email, and password are required"}
}
passwordHash, err := auth.HashPassword(password)
if err != nil {
return nil, fmt.Errorf("hash password: %w", err)
return nil, model.NewInternalError("hash password", err)
}
user := &model.User{
@@ -76,9 +76,9 @@ func (s *AuthService) Register(ctx context.Context, username, email, password st
if err := s.userRepo.Create(ctx, user); err != nil {
if errors.Is(err, model.ErrDuplicate) {
return nil, fmt.Errorf("username or email already exists")
return nil, &model.AppError{Status: http.StatusConflict, Message: "username or email already exists"}
}
return nil, fmt.Errorf("create user: %w", err)
return nil, model.NewInternalError("create user", err)
}
return user, nil
@@ -89,13 +89,13 @@ func (s *AuthService) Login(ctx context.Context, email, password string) (*Token
user, err := s.userRepo.FindByEmail(ctx, email)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("invalid email or password")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid email or password"}
}
return nil, fmt.Errorf("find user: %w", err)
return nil, model.NewInternalError("find user", err)
}
if err := auth.VerifyPassword(user.PasswordHash, password); err != nil {
return nil, fmt.Errorf("invalid email or password")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid email or password"}
}
return s.issueTokens(ctx, user.ID)
@@ -106,28 +106,28 @@ func (s *AuthService) Login(ctx context.Context, email, password string) (*Token
func (s *AuthService) Refresh(ctx context.Context, refreshTokenStr string) (*TokenPair, error) {
claims, err := auth.ParseToken(refreshTokenStr, s.jwtSecret)
if err != nil {
return nil, fmt.Errorf("invalid token")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid token"}
}
if claims.Type != auth.TokenRefresh {
return nil, fmt.Errorf("invalid token")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid token"}
}
tokenHash := auth.HashToken(refreshTokenStr)
session, err := s.sessionRepo.FindByTokenHash(ctx, tokenHash)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("invalid token")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid token"}
}
return nil, fmt.Errorf("find session: %w", err)
return nil, model.NewInternalError("find session", err)
}
if session.UserID != claims.UserID {
return nil, fmt.Errorf("invalid token")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid token"}
}
if err := s.sessionRepo.Delete(ctx, session.ID); err != nil {
return nil, fmt.Errorf("delete old session: %w", err)
return nil, model.NewInternalError("delete old session", err)
}
return s.issueTokens(ctx, claims.UserID)
@@ -141,7 +141,7 @@ func (s *AuthService) Logout(ctx context.Context, refreshTokenStr string) error
if errors.Is(err, model.ErrNotFound) {
return nil
}
return fmt.Errorf("find session: %w", err)
return model.NewInternalError("find session", err)
}
return s.sessionRepo.Delete(ctx, session.ID)
@@ -151,7 +151,7 @@ func (s *AuthService) Logout(ctx context.Context, refreshTokenStr string) error
func (s *AuthService) CreatePasskey(ctx context.Context, userID, label string) (*CreatedPasskey, error) {
raw, hash, err := auth.GenerateToken()
if err != nil {
return nil, fmt.Errorf("generate token: %w", err)
return nil, model.NewInternalError("generate token", err)
}
cred := &model.Credential{
@@ -163,7 +163,7 @@ func (s *AuthService) CreatePasskey(ctx context.Context, userID, label string) (
}
if err := s.credentialRepo.Create(ctx, cred); err != nil {
return nil, fmt.Errorf("create credential: %w", err)
return nil, model.NewInternalError("create credential", err)
}
return &CreatedPasskey{
@@ -176,24 +176,24 @@ func (s *AuthService) CreatePasskey(ctx context.Context, userID, label string) (
// LoginWithPasskey authenticates a user using an app passkey token.
func (s *AuthService) LoginWithPasskey(ctx context.Context, tokenStr string) (*TokenPair, error) {
if !strings.HasPrefix(tokenStr, "mygo_") {
return nil, fmt.Errorf("invalid passkey format")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid passkey format"}
}
tokenHash := auth.HashToken(tokenStr)
cred, err := s.credentialRepo.FindByHash(ctx, tokenHash)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("invalid passkey")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid passkey"}
}
return nil, fmt.Errorf("find credential: %w", err)
return nil, model.NewInternalError("find credential", err)
}
if cred.Type != "app_passkey" {
return nil, fmt.Errorf("invalid credential type")
return nil, &model.AppError{Status: http.StatusUnauthorized, Message: "invalid credential type"}
}
if err := s.credentialRepo.UpdateLastUsed(ctx, cred.ID); err != nil {
return nil, fmt.Errorf("update last used: %w", err)
return nil, model.NewInternalError("update last used", err)
}
return s.issueTokens(ctx, cred.UserID)
@@ -208,11 +208,14 @@ func (s *AuthService) ListPasskeys(ctx context.Context, userID string) ([]model.
func (s *AuthService) RevokePasskey(ctx context.Context, userID, credID string) error {
cred, err := s.credentialRepo.FindByID(ctx, credID)
if err != nil {
return fmt.Errorf("find credential: %w", err)
if errors.Is(err, model.ErrNotFound) {
return &model.AppError{Status: http.StatusNotFound, Message: "passkey not found", Err: model.ErrNotFound}
}
return model.NewInternalError("find credential", err)
}
if cred.UserID != userID {
return model.ErrForbidden
return &model.AppError{Status: http.StatusForbidden, Message: "access denied", Err: model.ErrForbidden}
}
return s.credentialRepo.Delete(ctx, credID)
@@ -221,12 +224,12 @@ func (s *AuthService) RevokePasskey(ctx context.Context, userID, credID string)
func (s *AuthService) issueTokens(ctx context.Context, userID string) (*TokenPair, error) {
accessToken, err := auth.GenerateAccessToken(userID, s.jwtSecret, s.accessTTL)
if err != nil {
return nil, fmt.Errorf("generate access token: %w", err)
return nil, model.NewInternalError("generate access token", err)
}
refreshToken, err := auth.GenerateRefreshToken(userID, s.jwtSecret, s.refreshTTL)
if err != nil {
return nil, fmt.Errorf("generate refresh token: %w", err)
return nil, model.NewInternalError("generate refresh token", err)
}
session := &model.Session{
@@ -237,7 +240,7 @@ func (s *AuthService) issueTokens(ctx context.Context, userID string) (*TokenPai
}
if err := s.sessionRepo.Create(ctx, session); err != nil {
return nil, fmt.Errorf("create session: %w", err)
return nil, model.NewInternalError("create session", err)
}
return &TokenPair{
+5 -2
View File
@@ -2,6 +2,8 @@ package service
import (
"context"
"errors"
"net/http"
"testing"
"time"
@@ -336,8 +338,9 @@ func TestAuthService_RevokePasskeyNotOwner(t *testing.T) {
claimsBob, _ := auth.ParseToken(pairBob.AccessToken, []byte("test-secret"))
err = svc.RevokePasskey(ctx, claimsBob.UserID, pk.ID)
if err != model.ErrForbidden {
t.Fatalf("expected ErrForbidden, got %v", err)
var ae *model.AppError
if !errors.As(err, &ae) || ae.Status != http.StatusForbidden {
t.Fatalf("expected AppError 403 Forbidden, got %v", err)
}
}
+46 -39
View File
@@ -8,6 +8,8 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"net/http"
"strings"
"time"
@@ -44,6 +46,7 @@ type FileService struct {
fileRepo repository.FileRepository
storage storage.StorageBackend
maxUploadSize int64
logger *slog.Logger
}
// NewFileService creates a FileService.
@@ -51,11 +54,16 @@ func NewFileService(
fileRepo repository.FileRepository,
storage storage.StorageBackend,
maxUploadSize int64,
logger *slog.Logger,
) *FileService {
if logger == nil {
logger = slog.Default()
}
return &FileService{
fileRepo: fileRepo,
storage: storage,
maxUploadSize: maxUploadSize,
logger: logger,
}
}
@@ -74,9 +82,9 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
// Check for name conflicts in the target directory.
if _, err := s.fileRepo.FindByNameAndParent(ctx, userID, parentID, fileName); err == nil {
return nil, fmt.Errorf("a file with this name already exists in this directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"}
} else if !errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("check name conflict: %w", err)
return nil, model.NewInternalError("check name conflict", err)
}
// Limit the reader if a max upload size is configured.
@@ -88,7 +96,7 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
head := make([]byte, 512)
n, readErr := io.ReadFull(reader, head)
if readErr != nil && readErr != io.ErrUnexpectedEOF && readErr != io.EOF {
return nil, fmt.Errorf("read for mime detection: %w", readErr)
return nil, model.NewInternalError("read for mime detection", readErr)
}
head = head[:n]
mimeType := mimetype.Detect(head).String()
@@ -103,13 +111,13 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
written, err := s.storage.Save(ctx, storagePath, teeReader)
if err != nil {
return nil, fmt.Errorf("save file: %w", err)
return nil, model.NewInternalError("save file", err)
}
if s.maxUploadSize > 0 && written > s.maxUploadSize {
// Clean up the oversized file.
_ = s.storage.Delete(ctx, storagePath)
return nil, fmt.Errorf("file size exceeds the maximum allowed size of %d bytes", s.maxUploadSize)
return nil, &model.AppError{Status: http.StatusBadRequest, Message: fmt.Sprintf("file size exceeds the maximum allowed size of %d bytes", s.maxUploadSize)}
}
file := &model.File{
@@ -128,9 +136,9 @@ func (s *FileService) Upload(ctx context.Context, userID string, parentID *strin
// Compensate: remove the stored file.
_ = s.storage.Delete(ctx, storagePath)
if errors.Is(err, model.ErrDuplicate) {
return nil, fmt.Errorf("a file with this name already exists in this directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"}
}
return nil, fmt.Errorf("create file record: %w", err)
return nil, model.NewInternalError("create file record", err)
}
return modelToFileInfo(file), nil
@@ -145,12 +153,12 @@ func (s *FileService) Download(ctx context.Context, userID, fileID string) (io.R
}
if file.IsDir {
return nil, nil, fmt.Errorf("cannot download a directory")
return nil, nil, &model.AppError{Status: http.StatusBadRequest, Message: "cannot download a directory"}
}
reader, err := s.storage.Open(ctx, file.StoragePath)
if err != nil {
return nil, nil, fmt.Errorf("open file: %w", err)
return nil, nil, model.NewInternalError("open file", err)
}
return reader, modelToFileInfo(file), nil
@@ -175,7 +183,7 @@ func (s *FileService) List(ctx context.Context, userID string, parentID *string,
files, total, err := s.fileRepo.FindByParentIDPaginated(ctx, userID, parentID, offset, limit)
if err != nil {
return nil, fmt.Errorf("list files: %w", err)
return nil, model.NewInternalError("list files", err)
}
infos := make([]FileInfo, 0, len(files))
@@ -204,7 +212,7 @@ func (s *FileService) Update(ctx context.Context, userID, fileID string, newName
// If parent is changing, verify the new parent.
if newParentID != nil {
if *newParentID == fileID {
return nil, fmt.Errorf("cannot move a directory into itself")
return nil, &model.AppError{Status: http.StatusBadRequest, Message: "cannot move a directory into itself"}
}
if err := s.verifyParent(ctx, userID, *newParentID); err != nil {
return nil, err
@@ -216,17 +224,17 @@ func (s *FileService) Update(ctx context.Context, userID, fileID string, newName
if newName != "" || newParentID != nil {
conflict, err := s.fileRepo.FindByNameAndParent(ctx, userID, file.ParentID, file.Name)
if err == nil && conflict.ID != fileID {
return nil, fmt.Errorf("a file with this name already exists in the target directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in the target directory"}
} else if err != nil && !errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("check name conflict: %w", err)
return nil, model.NewInternalError("check name conflict", err)
}
}
if err := s.fileRepo.Update(ctx, file); err != nil {
if errors.Is(err, model.ErrDuplicate) {
return nil, fmt.Errorf("a file with this name already exists in the target directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in the target directory"}
}
return nil, fmt.Errorf("update file: %w", err)
return nil, model.NewInternalError("update file", err)
}
return modelToFileInfo(file), nil
@@ -243,23 +251,22 @@ func (s *FileService) Delete(ctx context.Context, userID, fileID string) error {
if file.IsDir {
_, total, err := s.fileRepo.FindByParentIDPaginated(ctx, userID, &fileID, 0, 1)
if err != nil {
return fmt.Errorf("check directory contents: %w", err)
return model.NewInternalError("check directory contents", err)
}
if total > 0 {
return fmt.Errorf("directory is not empty")
return &model.AppError{Status: http.StatusConflict, Message: "directory is not empty"}
}
}
if err := s.fileRepo.Delete(ctx, fileID); err != nil {
return fmt.Errorf("delete file record: %w", err)
return model.NewInternalError("delete file record", err)
}
// Remove content from storage (directories have no stored content).
if !file.IsDir {
if err := s.storage.Delete(ctx, file.StoragePath); err != nil {
// Log but don't fail — the DB record is already gone.
// A periodic cleanup job can handle orphaned files later.
_ = err
s.logger.WarnContext(ctx, "failed to delete file from storage",
"path", file.StoragePath, "error", err)
}
}
@@ -280,24 +287,24 @@ func (s *FileService) CreateDir(ctx context.Context, userID string, parentID *st
// Check for name conflicts in the target directory.
if _, err := s.fileRepo.FindByNameAndParent(ctx, userID, parentID, dirName); err == nil {
return nil, fmt.Errorf("a file with this name already exists in this directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"}
} else if !errors.Is(err, model.ErrNotFound) {
return nil, fmt.Errorf("check name conflict: %w", err)
return nil, model.NewInternalError("check name conflict", err)
}
dir := &model.File{
ID: uuid.NewString(),
UserID: userID,
ID: uuid.NewString(),
UserID: userID,
ParentID: parentID,
Name: dirName,
IsDir: true,
Name: dirName,
IsDir: true,
}
if err := s.fileRepo.Create(ctx, dir); err != nil {
if errors.Is(err, model.ErrDuplicate) {
return nil, fmt.Errorf("a file with this name already exists in this directory")
return nil, &model.AppError{Status: http.StatusConflict, Message: "a file with this name already exists in this directory"}
}
return nil, fmt.Errorf("create directory record: %w", err)
return nil, model.NewInternalError("create directory record", err)
}
return modelToFileInfo(dir), nil
@@ -308,13 +315,13 @@ func (s *FileService) getOwnedFile(ctx context.Context, userID, fileID string) (
file, err := s.fileRepo.FindByID(ctx, fileID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
return nil, model.ErrNotFound
return nil, &model.AppError{Status: http.StatusNotFound, Message: "file not found", Err: model.ErrNotFound}
}
return nil, fmt.Errorf("find file: %w", err)
return nil, model.NewInternalError("find file", err)
}
if file.UserID != userID {
return nil, model.ErrForbidden
return nil, &model.AppError{Status: http.StatusForbidden, Message: "access denied", Err: model.ErrForbidden}
}
return file, nil
@@ -325,17 +332,17 @@ func (s *FileService) verifyParent(ctx context.Context, userID, parentID string)
parent, err := s.fileRepo.FindByID(ctx, parentID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
return fmt.Errorf("parent directory not found")
return &model.AppError{Status: http.StatusNotFound, Message: "parent directory not found"}
}
return fmt.Errorf("find parent: %w", err)
return model.NewInternalError("find parent", err)
}
if parent.UserID != userID {
return model.ErrForbidden
return &model.AppError{Status: http.StatusForbidden, Message: "access denied", Err: model.ErrForbidden}
}
if !parent.IsDir {
return fmt.Errorf("parent is not a directory")
return &model.AppError{Status: http.StatusBadRequest, Message: "parent is not a directory"}
}
return nil
@@ -345,13 +352,13 @@ func (s *FileService) verifyParent(ctx context.Context, userID, parentID string)
// null bytes, or are reserved names "." and "..".
func validateFileName(name string) error {
if name == "" {
return fmt.Errorf("file name must not be empty")
return &model.AppError{Status: http.StatusBadRequest, Message: "file name must not be empty"}
}
if strings.ContainsAny(name, "/\\\x00") {
return fmt.Errorf("file name contains invalid characters")
return &model.AppError{Status: http.StatusBadRequest, Message: "file name contains invalid characters"}
}
if name == "." || name == ".." {
return fmt.Errorf("file name is reserved: %s", name)
return &model.AppError{Status: http.StatusBadRequest, Message: fmt.Sprintf("file name is reserved: %s", name)}
}
return nil
}
+24 -19
View File
@@ -3,7 +3,9 @@ package service
import (
"bytes"
"context"
"errors"
"io"
"net/http"
"strings"
"sync"
"testing"
@@ -16,6 +18,21 @@ import (
"github.com/dhao2001/mygo/internal/storage"
)
// assertAppError checks that err is an *model.AppError with the expected status.
func assertAppError(t *testing.T, err error, wantStatus int) bool {
t.Helper()
var ae *model.AppError
if !errors.As(err, &ae) {
t.Errorf("expected *model.AppError, got %T: %v", err, err)
return false
}
if ae.Status != wantStatus {
t.Errorf("status = %d, want %d; message = %q", ae.Status, wantStatus, ae.Message)
return false
}
return true
}
// memStorage is an in-memory implementation of storage.StorageBackend for tests.
type memStorage struct {
mu sync.RWMutex
@@ -70,7 +87,7 @@ func setupFileService(t *testing.T) *FileService {
fileRepo := repository.NewFileRepository(db)
store := newMemStorage()
return NewFileService(fileRepo, store, 0) // 0 = unlimited
return NewFileService(fileRepo, store, 0, nil) // 0 = unlimited, nil logger defaults to slog.Default()
}
func TestFileService_Upload(t *testing.T) {
@@ -214,9 +231,7 @@ func TestFileService_DownloadForbidden(t *testing.T) {
}
_, _, err = svc.Download(ctx, "user2", info.ID)
if err != model.ErrForbidden {
t.Errorf("expected ErrForbidden, got %v", err)
}
assertAppError(t, err, http.StatusForbidden)
}
func TestFileService_DownloadDirectory(t *testing.T) {
@@ -257,9 +272,7 @@ func TestFileService_GetNotFound(t *testing.T) {
ctx := context.Background()
_, err := svc.Get(ctx, "user1", "nonexistent")
if err != model.ErrNotFound {
t.Errorf("expected ErrNotFound, got %v", err)
}
assertAppError(t, err, http.StatusNotFound)
}
func TestFileService_GetForbidden(t *testing.T) {
@@ -272,9 +285,7 @@ func TestFileService_GetForbidden(t *testing.T) {
}
_, err = svc.Get(ctx, "user2", info.ID)
if err != model.ErrForbidden {
t.Errorf("expected ErrForbidden, got %v", err)
}
assertAppError(t, err, http.StatusForbidden)
}
func TestFileService_List(t *testing.T) {
@@ -372,9 +383,7 @@ func TestFileService_Delete(t *testing.T) {
}
_, err = svc.Get(ctx, "user1", info.ID)
if err != model.ErrNotFound {
t.Errorf("expected ErrNotFound after delete, got %v", err)
}
assertAppError(t, err, http.StatusNotFound)
}
func TestFileService_DeleteNonEmptyDirectory(t *testing.T) {
@@ -406,9 +415,7 @@ func TestFileService_DeleteForbidden(t *testing.T) {
}
err = svc.Delete(ctx, "user2", info.ID)
if err != model.ErrForbidden {
t.Errorf("expected ErrForbidden, got %v", err)
}
assertAppError(t, err, http.StatusForbidden)
}
func TestFileService_CreateDir(t *testing.T) {
@@ -452,7 +459,5 @@ func TestFileService_VerifyParentForbidden(t *testing.T) {
}
_, err = svc.Upload(ctx, "user2", &dir.ID, "stolen.txt", strings.NewReader("data"))
if err != model.ErrForbidden {
t.Errorf("expected ErrForbidden, got %v", err)
}
assertAppError(t, err, http.StatusForbidden)
}