Code Review Horror Stories. Часть 2: API, ошибки и graceful shutdown

func (r *InMemoryClickRepository) GetRecent(
    userUUID, campaignUUID string,
    maxAge time.Duration,
) (error, *CampaignClick, bool) {
    // ...
}

val, err := someFunc()

err, click, ok := repo.GetRecent(...)

func (r *InMemoryClickRepository) GetRecent(
    userUUID, campaignUUID string,
    maxAge time.Duration,
) (*CampaignClick, bool) {
    // если error реально не возвращается — убираем его
}

c.repo.Save(click)
go c.publisher.Publish(click)
ctx.JSON(http.StatusOK, ClickResponse{
    ClickUUID:    click.ClickUUID,
    TrackingLink: click.TrackingLink,
})

if err := c.repo.Save(ctx.Request.Context(), click); err != nil {
    if dbErr, ok := errors.AsType[*DBError](err); ok && dbErr.Retriable {
        // retry или fallback
    }
    log.Printf("save click failed: %v", err)
    ctx.JSON(http.StatusInternalServerError, gin.H{"error": "internal"})
    return
}

func (c *ClickController) Post(ctx *gin.Context) {
    var req struct {
        UserUUID     string `json:"user_uuid"`
        CampaignUUID string `json:"campaign_uuid"`
        Country      string `json:"country"`
    }
    _ = ctx.ShouldBindJSON(&req)
    // дальше используем req...
}

func (c *ClickController) Post(ctx *gin.Context) {
    var req struct {
        UserUUID     string `json:"user_uuid"     binding:"required,uuid"`
        CampaignUUID string `json:"campaign_uuid" binding:"required,uuid"`
        Country      string `json:"country"       binding:"required,len=2,alpha"`
    }
    if err := ctx.ShouldBindJSON(&req); err != nil {
        ctx.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
        return
    }
    // дальше можно доверять req
}

import "github.com/biter777/countries"
func validateCountry(fl validator.FieldLevel) bool {
    code := fl.Field().String()
    return countries.ByName(code).IsValid()
}
// при инициализации:
if v, ok := binding.Validator.Engine().(*validator.Validate); ok {
    v.RegisterValidation("iso3166", validateCountry)
}
// в struct'е:
Country string `binding:"required,iso3166"`

type CountryCode string
func (c *CountryCode) UnmarshalJSON(data []byte) error {
    var s string
    if err := json.Unmarshal(data, &s); err != nil {
        return err
    }
    if !isValidISO3166(s) {
        return fmt.Errorf("invalid country code: %q", s)
    }
    *c = CountryCode(s)
    return nil
}

func generateTrackingLink(campaignUUID, userUUID string) string {
    return "https://tracker.example/click?campaign=" + campaignUUID + "?user=" + userUUID
}

?campaign=abc&user=evil&fake=injected

import "net/url"
func generateTrackingLink(campaignUUID, userUUID string) string {
    u, _ := url.Parse("https://tracker.example/click")
    q := u.Query()
    q.Set("campaign", campaignUUID)
    q.Set("user", userUUID)
    u.RawQuery = q.Encode()  // автоматический escape
    return u.String()
}

ctx.JSON(http.StatusOK, ClickResponse{...})

• Новый клик → создан ресурс → 201 Created
  
• Дедуплицированный → старый ресурс → 200 OK (или даже 409 Conflict, если хотим явно сигналить дубль)

if cached, ok := c.repo.GetRecent(...); ok {
    ctx.JSON(http.StatusOK, toResponse(cached))
    return
}
// ... создание нового
ctx.JSON(http.StatusCreated, toResponse(click))

func (r *InMemoryClickRepository) Save(ctx context.Context, c *CampaignClick) error {
    select {
    case <-ctx.Done():
        return ctx.Err()
    default:
    }
    r.mu.Lock()
    defer r.mu.Unlock()
    r.clicks = append(r.clicks, c)
    return nil
}

if err := c.repo.Save(ctx.Request.Context(), click); err != nil {
    // ...
}

r := gin.Default()
r.POST("/v1/campaign/click", ctrl.Post)
if err := r.Run(":8080"); err != nil {
    panic(err)
}

srv := &http.Server{
    Addr:              ":8080",
    Handler:           r,
    ReadHeaderTimeout: 5 * time.Second,  // защита от Slowloris
    ReadTimeout:       30 * time.Second,
    WriteTimeout:      30 * time.Second,
    IdleTimeout:       120 * time.Second,
}
go func() {
    if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
        log.Fatalf("server: %v", err)
    }
}()
// Ждём сигнал
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
<-quit
// Даём 30 секунд на завершение in-flight запросов
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := srv.Shutdown(ctx); err != nil {
    log.Printf("forced shutdown: %v", err)
}
// И только теперь закрываем publisher, repository и т.д.
publisher.Close(ctx)

• Один медленный клиент держит соединение бесконечно (атака Slowloris)
  
• Каждое такое соединение — занятый worker
  
• На N соединений сервис ложится при N == file descriptor limit

srv := &http.Server{
    ReadHeaderTimeout: 5 * time.Second,   // обязательно — защита от Slowloris
    ReadTimeout:       30 * time.Second,  // полный read с body
    WriteTimeout:      30 * time.Second,  // запись ответа
    IdleTimeout:       120 * time.Second, // keep-alive
}

log.Println("post error:", err)
log.Println("click:", string(bodyBytes))

• Не структурировано — в production система мониторинга (Loki, Datadog, ELK) не сможет нормально парсить
  
• Нет уровней — нельзя отфильтровать DEBUG от ERROR
  
• Нет контекста — нет request_id, trace_id, user_id
  
• Глобальный logger — нельзя подменить на тестах

import "log/slog"
logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{
    Level: slog.LevelInfo,
}))
logger.Error("publish failed",
    slog.String("click_uuid", click.ClickUUID),
    slog.String("user_uuid", click.UserUUID),
    slog.Any("err", err),
)

{"time":"2026-05-04T12:00:00Z","level":"ERROR","msg":"publish failed","click_uuid":"...","user_uuid":"...","err":"..."}

go func() {
    ticker := time.NewTicker(1 * time.Second)
    defer ticker.Stop()
    for range ticker.C {
        payload := struct {
            UserUUID     string `json:"user_uuid"`
            CampaignUUID string `json:"campaign_uuid"`
            Country      string `json:"country"`
        }{
            UserUUID:     uuid.NewString(),  // ❌ новый UUID каждый раз!
            CampaignUUID: uuid.NewString(),  // ❌ новый UUID каждый раз!
            Country:      "US",
        }
        b, _ := json.Marshal(payload)
        body := bytes.NewBuffer(b)
        resp, err := http.Post("http://localhost:8080/v1/campaign/click", "application/json", body)
        if err != nil {
            log.Println("post error:", err)
            continue
        }
        bodyBytes, _ := io.ReadAll(resp.Body)
        log.Println("click:", string(bodyBytes))
        _ = resp.Body.Close()
    }
}()

testUsers := []string{uuid.NewString(), uuid.NewString(), uuid.NewString()}
testCampaigns := []string{uuid.NewString(), uuid.NewString()}
httpClient := &http.Client{
    Timeout: 5 * time.Second,
}
go func() {
    // Ждём, пока сервер поднимется
    time.Sleep(500 * time.Millisecond)
    ticker := time.NewTicker(1 * time.Second)
    defer ticker.Stop()
    i := 0
    for range ticker.C {
        payload := struct{ /* ... */ }{
            UserUUID:     testUsers[i%len(testUsers)],         // повторы → дедуп сработает!
            CampaignUUID: testCampaigns[i%len(testCampaigns)],
            Country:      "US",
        }
        b, err := json.Marshal(payload)
        if err != nil {
            slog.Error("marshal", "err", err)
            continue
        }
        resp, err := httpClient.Post(
            "http://localhost:8080/v1/campaign/click",
            "application/json",
            bytes.NewBuffer(b),
        )
        if err != nil {
            slog.Error("post", "err", err)
            continue
        }
        bodyBytes, err := io.ReadAll(resp.Body)
        resp.Body.Close()
        if err != nil {
            slog.Error("read body", "err", err)
            continue
        }
        slog.Info("click", "response", string(bodyBytes))
        i++
    }
}()

• errors.AsType[T] — типобезопасный generic-вариант errors.As. Чище парсятся доменные ошибки (см. баг #11).
  
• io.ReadAll×2 быстрее, −50% памяти — те, кто читает HTTP body, получают бесплатный буст (см. баг #21).
  
• HTTP/2StrictMaxConcurrentRequests — теперь можно нормально ограничивать concurrent requests на уровне транспорта.
  
• HTTP 307 для trailing slash redirects (вместо 301) — breaking change, проверьте редиректы в API.
  
• net/url.Parseстал строже к malformed URL’ам с двоеточиями в host. Если что-то сломалось — GODEBUG=urlstrictcolons=0 вернёт старое поведение.

• Забытый Lock в Save (data race)
  
• Канал без consumer’а (goroutine leak)
  
• Publisher по значению (channel duplication)
  
• PurgeOld не вызывается (memory leak)
  
• PurgeOld O(N²) под Lock (latency spike)
  
• TOCTOU race в дедупликации
  
• O(N) поиск в горячем пути
  
• go publisher.Publish() без context
  API & Reliability (Часть 2):
  
• error не последний в return
  
• Save() error игнорируется
  
• Нет валидации входных данных
  
• Country не проверяется на ISO 3166-1
  
• Битый URL (два ?)
  
• URL без escaping
  
• Status 200 вместо 201
  
• Нет context.Context
  
• Нет graceful shutdown
  
• Нет HTTP timeout’ов на сервере
  
• log.Println вместо slog
  
• Тестовая горутина бесполезна (random UUIDs + http.Post без timeout)

• Compilation: package, импорты, объявления (это часто пропускают, и зря)
  
• Concurrency: где Lock? где race? где забытый channel close?
  
• Resource leaks: горутины, каналы, файлы, connections, память
  
• Error handling: где проигнорированы errors? где nil panic’нет?
  
• Boundaries: валидация на input, escaping на output
  
• API design: status codes, error returns, context propagation
  
• Reliability: shutdown, timeouts, retries, observability
  
• Performance hot paths: O(N) в handler — это плохо

• GMP Scheduler (готов!)
  
• Memory Model & Escape Analysis (готов!)
  
• Garbage Collector Internals
  
• Interfaces & Reflection
  
• Generics in Production
  
• Context & Error Patterns