Конвенция для обработки ошибок + рефакторинг кода
This commit is contained in:
+67
-14
@@ -8,6 +8,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"html/template"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
@@ -20,7 +21,9 @@ import (
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
|
||||
"git.vakhrushev.me/av/jellybit/internal/ingest"
|
||||
"git.vakhrushev.me/av/jellybit/internal/magnet"
|
||||
"git.vakhrushev.me/av/jellybit/internal/store"
|
||||
"git.vakhrushev.me/av/jellybit/internal/worker"
|
||||
"git.vakhrushev.me/av/jellybit/web"
|
||||
)
|
||||
|
||||
@@ -154,12 +157,12 @@ func (s *server) handleUIAdd(w http.ResponseWriter, r *http.Request) {
|
||||
redirectErr(w, r, "не удалось разобрать форму")
|
||||
return
|
||||
}
|
||||
_, err := s.deps.Ingestor.Ingest(r.Context(), ingest.Request{
|
||||
res, err := s.deps.Ingestor.Ingest(r.Context(), ingest.Request{
|
||||
Source: r.PostForm.Get("source"),
|
||||
Context: r.PostForm.Get("context"),
|
||||
})
|
||||
if err != nil {
|
||||
redirectErr(w, r, err.Error())
|
||||
redirectErr(w, r, userErr(r, err, res.DownloadID))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -172,7 +175,7 @@ func (s *server) handleUICancel(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if err := s.deps.Commander.Cancel(r.Context(), id); err != nil {
|
||||
redirectErr(w, r, err.Error())
|
||||
redirectErr(w, r, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -207,7 +210,7 @@ type addResponse struct {
|
||||
func (s *server) handleAPIList(w http.ResponseWriter, r *http.Request) {
|
||||
downloads, err := s.deps.Reader.ListDownloads(r.Context())
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, errJSON(err))
|
||||
s.apiErr(w, r, err, 0)
|
||||
return
|
||||
}
|
||||
out := make([]downloadDTO, 0, len(downloads))
|
||||
@@ -220,12 +223,13 @@ func (s *server) handleAPIList(w http.ResponseWriter, r *http.Request) {
|
||||
func (s *server) handleAPIGet(w http.ResponseWriter, r *http.Request) {
|
||||
id, err := pathID(r)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, errJSON(err))
|
||||
writeJSON(w, http.StatusBadRequest, errBody(r, "некорректный id", 0))
|
||||
return
|
||||
}
|
||||
d, err := s.deps.Reader.GetDownload(r.Context(), id)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusNotFound, errJSON(err))
|
||||
// ErrNotFound → 404, реальный сбой БД → 500 (не маскируем под 404).
|
||||
s.apiErr(w, r, err, id)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, toDTO(*d))
|
||||
@@ -234,12 +238,15 @@ func (s *server) handleAPIGet(w http.ResponseWriter, r *http.Request) {
|
||||
func (s *server) handleAPIAdd(w http.ResponseWriter, r *http.Request) {
|
||||
var req addRequest
|
||||
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<16)).Decode(&req); err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, errJSON(err))
|
||||
writeJSON(w, http.StatusBadRequest, errBody(r, "некорректный запрос", 0))
|
||||
return
|
||||
}
|
||||
res, err := s.deps.Ingestor.Ingest(r.Context(), ingest.Request{Source: req.Source, Context: req.Context})
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, errJSON(err))
|
||||
// res.DownloadID непуст, если сбой после создания задачи (напр. qbit) —
|
||||
// тогда коррелируем по download_id, иначе (ранний разбор источника) по
|
||||
// request_id.
|
||||
s.apiErr(w, r, err, res.DownloadID)
|
||||
return
|
||||
}
|
||||
status := http.StatusCreated
|
||||
@@ -265,14 +272,14 @@ func (s *server) handleAPIRetry(w http.ResponseWriter, r *http.Request) {
|
||||
func (s *server) apiCommand(w http.ResponseWriter, r *http.Request, cmd func(context.Context, int64) error) {
|
||||
id, err := pathID(r)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, errJSON(err))
|
||||
writeJSON(w, http.StatusBadRequest, errBody(r, "некорректный id", 0))
|
||||
return
|
||||
}
|
||||
if err := cmd(r.Context(), id); err != nil {
|
||||
// Тонкий транспорт: возвращённую use-case'ом/воркером ошибку переводим в
|
||||
// HTTP-статус и не логируем повторно (доменный слой уже залогировал, а
|
||||
// невалидный ввод — это норма, разбирать команде нечего).
|
||||
writeJSON(w, http.StatusConflict, errJSON(err))
|
||||
// статус+сообщение и не логируем повторно (доменный слой уже залогировал,
|
||||
// а невалидный ввод — норма, разбирать команде нечего).
|
||||
s.apiErr(w, r, err, id)
|
||||
return
|
||||
}
|
||||
d, err := s.deps.Reader.GetDownload(r.Context(), id)
|
||||
@@ -339,8 +346,54 @@ func writeJSON(w http.ResponseWriter, status int, v any) {
|
||||
_ = json.NewEncoder(w).Encode(v)
|
||||
}
|
||||
|
||||
func errJSON(err error) map[string]string {
|
||||
return map[string]string{"error": err.Error()}
|
||||
// classifyErr транслирует доменную ошибку в HTTP-статус и нейтральное
|
||||
// человекочитаемое сообщение публичного канала (без сырого err.Error() и
|
||||
// деталей реализации): ErrNotFound → 404, валидация источника
|
||||
// (magnet.ErrNotMagnet) → 400, конфликт состояния (worker.ErrConflict) → 409,
|
||||
// прочее → 500. Полная ошибка уже в логах на доменной границе — наружу отдаём
|
||||
// только сообщение + корреляционный ключ.
|
||||
func classifyErr(err error) (int, string) {
|
||||
switch {
|
||||
case errors.Is(err, store.ErrNotFound):
|
||||
return http.StatusNotFound, "не найдено"
|
||||
case errors.Is(err, magnet.ErrNotMagnet):
|
||||
return http.StatusBadRequest, "некорректный источник"
|
||||
case errors.Is(err, worker.ErrConflict):
|
||||
// Нормальный конфликт состояния (операция недопустима сейчас), не сбой.
|
||||
return http.StatusConflict, "действие недоступно в текущем состоянии"
|
||||
default:
|
||||
return http.StatusInternalServerError, "внутренняя ошибка"
|
||||
}
|
||||
}
|
||||
|
||||
// errBody — тело ошибки REST API: нейтральное сообщение + корреляционный ключ
|
||||
// для владельца (download_id, если операция привязана к загрузке, иначе
|
||||
// request_id запроса), по которому он найдёт полную ошибку в логах.
|
||||
func errBody(r *http.Request, msg string, downloadID int64) map[string]any {
|
||||
body := map[string]any{"error": msg}
|
||||
if downloadID > 0 {
|
||||
body["download_id"] = downloadID
|
||||
} else {
|
||||
body["request_id"] = middleware.GetReqID(r.Context())
|
||||
}
|
||||
return body
|
||||
}
|
||||
|
||||
// apiErr пишет ответ об ошибке REST API по доменной ошибке (статус + тело).
|
||||
func (s *server) apiErr(w http.ResponseWriter, r *http.Request, err error, downloadID int64) {
|
||||
status, msg := classifyErr(err)
|
||||
writeJSON(w, status, errBody(r, msg, downloadID))
|
||||
}
|
||||
|
||||
// userErr — сообщение публичного канала для веб-UI: нейтральный текст по
|
||||
// доменной ошибке + корреляционный ключ владельцу (download_id, если операция
|
||||
// привязана к загрузке, иначе request_id). Сырой текст ошибки наружу не идёт.
|
||||
func userErr(r *http.Request, err error, downloadID int64) string {
|
||||
_, msg := classifyErr(err)
|
||||
if downloadID > 0 {
|
||||
return fmt.Sprintf("%s (download_id=%d)", msg, downloadID)
|
||||
}
|
||||
return fmt.Sprintf("%s (request_id=%s)", msg, middleware.GetReqID(r.Context()))
|
||||
}
|
||||
|
||||
// requestLogger пишет структурированный лог по каждому запросу. Частые
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
@@ -14,6 +15,7 @@ import (
|
||||
"git.vakhrushev.me/av/jellybit/internal/httpapi"
|
||||
"git.vakhrushev.me/av/jellybit/internal/ingest"
|
||||
"git.vakhrushev.me/av/jellybit/internal/layout"
|
||||
"git.vakhrushev.me/av/jellybit/internal/magnet"
|
||||
"git.vakhrushev.me/av/jellybit/internal/recognize"
|
||||
"git.vakhrushev.me/av/jellybit/internal/store"
|
||||
"git.vakhrushev.me/av/jellybit/internal/worker"
|
||||
@@ -104,7 +106,9 @@ func TestAPIAdd(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAPIAddBadInput(t *testing.T) {
|
||||
ing := &fakeIngestor{err: ingestErr("bad magnet")}
|
||||
// Источник не magnet → ingest оборачивает magnet.ErrNotMagnet; транспорт
|
||||
// классифицирует это как 400 (некорректный источник).
|
||||
ing := &fakeIngestor{err: fmt.Errorf("ingest: parse source: %w", magnet.ErrNotMagnet)}
|
||||
srv := newServer(t, httpapi.Deps{Ingestor: ing, Commander: &fakeCommander{}, Reader: &fakeReader{}})
|
||||
|
||||
resp, err := http.Post(srv.URL+"/api/downloads", "application/json", strings.NewReader(`{"source":"x"}`))
|
||||
@@ -156,6 +160,26 @@ func TestAPICancel(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPICommandConflict(t *testing.T) {
|
||||
// Конфликт состояния (worker.ErrConflict) → 409, не 500.
|
||||
cmd := &fakeCommander{err: fmt.Errorf("cancel: download 5 in wrong state: %w", worker.ErrConflict)}
|
||||
srv := newServer(t, httpapi.Deps{Ingestor: &fakeIngestor{}, Commander: cmd, Reader: &fakeReader{}})
|
||||
|
||||
resp, err := http.Post(srv.URL+"/api/downloads/5/cancel", "", nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusConflict {
|
||||
t.Fatalf("status = %d, want 409", resp.StatusCode)
|
||||
}
|
||||
var got map[string]any
|
||||
_ = json.NewDecoder(resp.Body).Decode(&got)
|
||||
if got["download_id"].(float64) != 5 {
|
||||
t.Errorf("download_id корреляции нет: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIndexRenders(t *testing.T) {
|
||||
reader := &fakeReader{list: []store.Download{
|
||||
{ID: 1, SourceType: store.SourceMagnet, SourceRef: "magnet:?xt=urn:btih:abc", State: store.StateDownloading},
|
||||
|
||||
@@ -2,12 +2,12 @@ package httpapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strconv"
|
||||
|
||||
"git.vakhrushev.me/av/jellybit/internal/store"
|
||||
"git.vakhrushev.me/av/jellybit/internal/worker"
|
||||
)
|
||||
|
||||
@@ -78,12 +78,12 @@ func (s *server) handleReview(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
rd, err := s.deps.Reviewer.ReviewData(r.Context(), id)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
if errors.Is(err, store.ErrNotFound) {
|
||||
http.Error(w, "задача не найдена", http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
s.deps.Logger.Error("review data", "id", id, "error", err)
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
http.Error(w, "внутренняя ошибка", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -155,7 +155,7 @@ func (s *server) handleApply(w http.ResponseWriter, r *http.Request) {
|
||||
if err := s.deps.Reviewer.Apply(r.Context(), id); err != nil {
|
||||
// Тонкий транспорт: ошибку воркера переводим в ответ, не логируя
|
||||
// повторно (доменный слой уже залогировал реальный сбой).
|
||||
redirectReview(w, r, id, err.Error())
|
||||
redirectReview(w, r, id, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -221,7 +221,7 @@ func (s *server) handleDefer(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if err := s.deps.Reviewer.Defer(r.Context(), id); err != nil {
|
||||
redirectReview(w, r, id, err.Error())
|
||||
redirectReview(w, r, id, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -234,7 +234,7 @@ func (s *server) handleUndo(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if err := s.deps.Reviewer.Undo(r.Context(), id); err != nil {
|
||||
redirectErr(w, r, err.Error())
|
||||
redirectErr(w, r, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -249,7 +249,7 @@ func (s *server) handleRelink(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if err := s.deps.Reviewer.Relink(r.Context(), id); err != nil {
|
||||
redirectErr(w, r, err.Error())
|
||||
redirectErr(w, r, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
@@ -266,7 +266,7 @@ func (s *server) reviewAction(w http.ResponseWriter, r *http.Request, fn func(co
|
||||
if err := fn(r.Context(), id); err != nil {
|
||||
// Тонкий транспорт: ошибку переводим в ?err= на странице ревью, не
|
||||
// логируя повторно (доменный слой/валидация — не дело транспорта).
|
||||
redirectReview(w, r, id, err.Error())
|
||||
redirectReview(w, r, id, userErr(r, err, id))
|
||||
return
|
||||
}
|
||||
redirectReview(w, r, id, "")
|
||||
|
||||
Reference in New Issue
Block a user