veza/veza-backend-api/AUDIT_POST_REMEDIATION.md
2025-12-12 21:34:34 -05:00

10 KiB

🔍 AUDIT TECHNIQUE POST-REMÉDIATION — VEZA BACKEND API

Date: 2025-12-12
Contexte: Module remédié (P0/P1 traités) — Identification des blocages résiduels pour production


A. ÉTAT GLOBAL DU MODULE

Verdict

Module prêt avec réserves — Les correctifs P0/P1 ont considérablement amélioré la stabilité et la sécurité. Cependant, 3 problèmes P1-résiduels et plusieurs P2-stability doivent être traités avant une mise en production confiante.

Points forts :

  • Sécurité critique (CORS, ownership, validation) : SOLIDE
  • Robustesse de base (timeout, readiness) : ACCEPTABLE
  • Tests critiques (ownership, validation) : COUVERTS

Points faibles :

  • ⚠️ Incohérences architecturales : Patterns d'erreur divergents
  • ⚠️ Résilience externe : Pas de retry/circuit breaker sur dépendances
  • ⚠️ Observabilité limitée : Métriques DB pool non exposées

B. PROBLÈMES RESTANTS

🔴 P1-RESIDUAL (Bloquant production)

P1-RES-001 : Incohérence patterns de réponse d'erreur

Description : Deux patterns d'erreur coexistent sans standardisation :

  • TrackHandler utilise response.BadRequest(), response.NotFound(), response.Success() (package internal/response)
  • ProfileHandler utilise RespondWithAppError(), RespondSuccess() (package handlers)

Impact :

  • Réponses API incohérentes selon l'endpoint
  • Difficulté de maintenance et debugging
  • Risque de confusion pour les clients API

Preuve :

// internal/core/track/handler.go:113
response.BadRequest(c, "no file provided")

// internal/handlers/profile_handler.go:52
RespondWithAppError(c, apperrors.New(apperrors.ErrCodeValidation, "invalid user id"))

Recommandation MINIMALE :

  1. Standardiser sur UN pattern (recommandé : RespondWithAppError + RespondSuccess du package handlers)
  2. Migrer TrackHandler vers le pattern standardisé
  3. Créer un helper centralisé si nécessaire

Fichiers concernés :

  • internal/core/track/handler.go (19 occurrences de response.*)
  • internal/handlers/profile_handler.go (utilise déjà le pattern standard)
  • internal/response/response.go (package à déprécier ou aligner)

P1-RES-002 : Absence de retry sur appels Stream Server

Description : Les appels HTTP vers Stream Server n'ont pas de mécanisme de retry. Un échec réseau temporaire provoque une perte de jobs de transcodage.

Note : WebhookService a déjà un retry implémenté (3 tentatives avec backoff exponentiel).

Impact :

  • Perte de jobs de transcodage si Stream Server temporairement indisponible
  • Pas de résilience face aux pannes partielles du Stream Server
  • État incohérent si le job est créé mais le Stream Server ne le reçoit pas

Preuve :

// internal/services/stream_service.go:55
resp, err := s.client.Do(req)
if err != nil {
    return fmt.Errorf("failed to send request: %w", err)  // Pas de retry
}

Recommandation MINIMALE :

  1. Ajouter retry avec backoff exponentiel (3 tentatives, max 30s) similaire à WebhookService
  2. Utiliser context.WithTimeout pour limiter le temps total
  3. Logger les échecs après retries épuisés

Fichiers concernés :

  • internal/services/stream_service.go

P1-RES-003 : c.MustGet() sans protection explicite

Description : Utilisation extensive de c.MustGet("user_id") qui peut panic si la clé n'existe pas. Bien que le middleware auth devrait toujours définir cette clé, une erreur de configuration ou un handler mal isolé peut provoquer un crash.

Impact :

  • Panic possible si middleware auth non appliqué
  • Pas de message d'erreur clair pour debugging
  • Risque de crash en production

Preuve :

// internal/core/track/handler.go:105 (et 18 autres occurrences)
userID := c.MustGet("user_id").(uuid.UUID)  // Peut panic

Recommandation MINIMALE :

  1. Créer un helper GetUserID(c *gin.Context) (uuid.UUID, error) qui retourne une erreur au lieu de panic
  2. Remplacer progressivement c.MustGet() par ce helper
  3. Ou au minimum : wrapper avec recover() dans un middleware de récupération (déjà présent mais pas idéal)

Fichiers concernés :

  • internal/core/track/handler.go (19 occurrences)
  • internal/handlers/profile_handler.go (utilise déjà c.Get() avec vérification)

🟠 P2-STABILITY (À traiter avant montée en charge)

P2-STAB-001 : Pas de circuit breaker sur dépendances externes

Description : Aucun circuit breaker implémenté pour protéger contre les dépendances lentes/indisponibles (Stream Server, Chat Server, Webhooks).

Impact :

  • Service peut être surchargé si dépendance lente
  • Pas de dégradation gracieuse
  • Timeouts peuvent s'accumuler

Recommandation : Implémenter un circuit breaker simple (ex: github.com/sony/gobreaker) pour :

  • Stream Server calls
  • Webhook deliveries
  • Chat Server health checks

Fichiers concernés :

  • internal/services/stream_service.go
  • internal/services/webhook_service.go
  • internal/handlers/status_handler.go

P2-STAB-002 : Pool stats DB non exposés dans métriques

Description : Les statistiques du pool de connexions DB (MaxOpenConns, OpenConns, InUse, Idle) ne sont pas exposées dans les métriques Prometheus/health.

Impact :

  • Impossible de diagnostiquer les problèmes de connexion en production
  • Pas de visibilité sur l'utilisation du pool
  • Difficulté à dimensionner correctement

Recommandation : Exposer les stats via :

  • Endpoint /metrics (Prometheus)
  • Endpoint /health (champ database.pool_stats)

Fichiers concernés :

  • internal/database/database.go (fonction GetPoolStats existe déjà)
  • internal/handlers/health.go

P2-STAB-003 : Migrations sans rollback global

Description : Chaque migration est transactionnelle (rollback si échec), mais si une migration échoue, les migrations précédentes restent appliquées. Pas de mécanisme de rollback global.

Impact :

  • DB peut être dans un état partiellement migré
  • Récupération manuelle nécessaire
  • Risque en production lors de déploiements

Recommandation :

  • Documenter la procédure de rollback manuel
  • Ajouter un script de vérification d'intégrité post-migration
  • (Optionnel) Implémenter un système de versioning de schéma avec rollback

Fichiers concernés :

  • internal/database/database.go (fonction Initialize())

P2-STAB-004 : Fichiers backup non nettoyés

Description : Dossiers .backup-pre-uuid-migration/ présents dans le codebase (migration UUID complétée).

Impact :

  • Confusion pour les développeurs
  • Risque d'utilisation accidentelle d'ancien code
  • Pollution du codebase

Recommandation : Supprimer les dossiers backup après vérification qu'ils ne sont plus référencés.

Fichiers concernés :

  • internal/handlers/.backup-pre-uuid-migration/
  • internal/services/.backup-pre-uuid-migration/
  • internal/models/.backup-pre-uuid-migration/

🟡 P3-CLEANUP (Acceptable avant prod)

P3-CLEAN-001 : TODOs restants dans le code

Description : Quelques TODOs/FIXMEs présents :

  • internal/core/track/handler.go:227 : "TODO(P2-GO-004): trackUploadService attend int64"
  • internal/core/track/service.go:225 : "TODO(P2-GO-018): Enqueue job pour traitement asynchrone"
  • internal/services/track_history_service.go:74 : "FIXME: models.TrackHistory needs UUID too"

Impact :

  • Dette technique mineure
  • Pas de blocage fonctionnel

Recommandation : Documenter ces TODOs et planifier leur traitement post-MVP.


P3-CLEAN-002 : Pas de versioning API

Description : Toutes les routes sont /api/v1/* mais pas de mécanisme de versioning pour breaking changes futurs.

Impact :

  • Difficulté à introduire des breaking changes
  • Pas de support multi-versions

Recommandation :

  • Documenter la stratégie de versioning
  • Préparer l'infrastructure pour /api/v2/* si nécessaire
  • (Non-bloquant pour MVP)

P3-CLEAN-003 : Tests manquants pour certains handlers

Description : Certains handlers n'ont pas de tests unitaires complets (ex: ChatHandler a des panic("not implemented") dans les tests).

Impact :

  • Couverture de tests incomplète
  • Risque de régression silencieuse

Recommandation :

  • Compléter les tests manquants progressivement
  • Prioriser les handlers critiques (auth, uploads, tracks)

Fichiers concernés :

  • internal/handlers/chat_handler_test.go

C. DÉCISION FINALE

Verdict

"Module prêt avec réserves"

Justification

Points positifs :

  • Sécurité critique solide (P0/P1 traités)
  • Tests critiques présents (ownership, validation)
  • Robustesse de base acceptable (timeout, readiness)

Blocages résiduels :

  • 🔴 P1-RES-001 : Incohérence patterns erreur (bloquant pour cohérence API)
  • 🔴 P1-RES-002 : Pas de retry Stream Server (bloquant pour résilience)
  • 🔴 P1-RES-003 : c.MustGet() non protégé (bloquant pour stabilité)

Recommandation :

  1. Corriger les 3 P1-RES avant production (estimation : 1-2 jours)
  2. Traiter les P2-STAB prioritaires (circuit breaker, pool stats) avant montée en charge
  3. P3-CLEAN peut être traité post-MVP

Plan d'action minimal

Phase 1 (Blocant prod) :

  1. Standardiser patterns d'erreur (P1-RES-001) — 4h
  2. Ajouter retry Stream Server (P1-RES-002) — 2h (réutiliser pattern WebhookService)
  3. Protéger c.MustGet() (P1-RES-003) — 2h

Phase 2 (Avant montée en charge) : 4. Circuit breaker dépendances (P2-STAB-001) — 4h 5. Exposer pool stats DB (P2-STAB-002) — 2h 6. Nettoyer fichiers backup (P2-STAB-004) — 30min

Total estimé : ~14h de travail


D. RÉSUMÉ EXÉCUTIF

Catégorie État Blocages
Sécurité Solide 0
Robustesse ⚠️ Acceptable 3 P1-RES
Observabilité ⚠️ Limité 1 P2-STAB
Tests Couvert (critiques) 0
Dette technique 🟡 Mineure 0

Conclusion : Module prêt pour phase CI/CD après correction des 3 P1-RES (estimation 1-2 jours).


Auditeur : AI Assistant
Date : 2025-12-12
Version : Post-remédiation P0/P1