maximus/Simpl-Resultat

Fork 0

db.ts : réserver le passage hors verrou au handle de la transaction #408

New issue

Closed

opened 2026-10-09 01:12:03 +00:00 by maximus · 1 comment

maximus commented

2026-10-09 01:12:03 +00:00

Owner

Planification du 2026-10-10 (/plan-run, milestone planned-2026-10-10-fiabilite-tests) : les sections ci-dessous, à partir de « Depends on », priment sur le texte d'analyse en cas d'écart.

Analyse /analyze du 2026-10-10, lue sur main 87920ec. Corps d'origine archivé en commentaire.

Contexte

Revue de #402 (commentaire 20659, point 1). Pendant le rappel d'un withTransaction, le drapeau global inTransaction de src/services/db.ts (L30-35) fait passer tout execute/select hors du verrou FIFO (serialize, L54-72), d'où qu'il vienne. Une requête lancée ailleurs pendant une transaction s'exécute donc en parallèle d'elle : deux opérations en vol, ce que le commentaire de tête de db.ts donne comme cause du « database is locked » de la 0.10.0. onboardingService.ts:128-131 le reconnaît déjà en commentaire.

Vérifié en analyse

Aucun rappel n'appelle getDb(), directement ou par une fonction appelée. Les 8 sites passent le handle reçu : balance.service.ts:710, :1509, :1597 ; onboardingService.ts:144 ; categoryMigrationService.ts:187 ; categorizationService.ts:227 ; dataExportService.ts:427 (runRestore, avec restoreCategories, restoreSuppliers, restoreKeywords, restoreImportTemplates, restoreImportSources, attachTransactions, qui prennent tous db en paramètre). Un verrou strict n'interbloque donc aucun chemin actuel.
Piège latent : findOrCreateSecurity (balance.service.ts:1006) fait const db = exec ?? (await getDb()). Un futur appel sans exec depuis un rappel interbloquerait sous la nouvelle règle.
Défaut voisin, inclus : connectToProfile (L163-167), initializeNewProfileDb (L195-204) et closeDb (L207-212) ferment rawInstance directement, sans attendre la file ni une transaction en cours. Un changement de profil (ProfileContext.tsx:97-98, :179-180) peut fermer la connexion au milieu d'une transaction. initializeNewProfileDb exécute aussi son SQL hors verrou. lockTail n'est jamais réinitialisé.
Le passage direct est voulu et épinglé par db.test.ts:106-117 (« lets a nested getDb() call run directly inside a transaction — no deadlock »), à remplacer.

Décisions (Max, 2026-10-10)

D1 — Verrou strict. Le rappel reçoit un handle propre à la transaction qui exécute directement ; tout autre appel (y compris getDb() depuis un rappel) attend derrière le verrou. Le drapeau global disparaît.
D2 — Handle invalidé à la fin du rappel. Après le finally, un appel sur ce handle lève une erreur nommée, au lieu de passer hors verrou depuis une promesse orpheline.
D3 — Fermeture par la file. connectToProfile, initializeNewProfileDb et closeDb ferment et rouvrent la connexion via runExclusive, donc après les opérations déjà en file et après une transaction en cours.

Travail à faire

db.ts : supprimer inTransaction ; withTransaction crée un proxy de transaction (exécution directe sur rawInstance capturé) passé au rappel, révoqué dans le finally.
db.ts : connectToProfile, initializeNewProfileDb, closeDb passent leur fermeture/ouverture (et le SQL d'initialisation) par runExclusive.
Mettre à jour le commentaire de tête et ceux de L30-35 et L50-52 : la règle d'interblocage devient « ne jamais appeler getDb() depuis un rappel, utiliser le handle reçu ».
balance.service.ts:1006 : rendre exec obligatoire sur findOrCreateSecurity (plus de exec ?? getDb()), appelants mis à jour.
Retirer le commentaire devenu faux de onboardingService.ts:128-131.
docs/architecture.md, section ### Accès DB et transactions (à créer) : la règle du verrou, du handle de transaction, de la génération et de l'alerte. Aucun ADR ne porte withTransaction (vérifié) : pas d'ADR.
Pas d'entrée CHANGELOG (robustesse interne, aucun comportement visible attendu).

Fichiers concernés

src/services/db.ts, src/services/db.test.ts
src/services/balance.service.ts (L1006)
src/services/onboardingService.ts (commentaire L128-131)
docs/architecture.md

Surface de test

db.test.ts a un Database factice (L48-64, 2 ms par appel, journal début/fin), réutilisable. Les tests de services mockent db.ts (withTransaction: fn => fn(await getDb())) et ne voient pas le verrou : ne pas s'y fier.
À ajouter dans db.test.ts :
- une lecture getDb().select lancée pendant le rappel (après BEGIN) se termine après le COMMIT (rouge aujourd'hui) ;
- le handle du rappel exécute sans attendre (pas d'interblocage sur ses propres requêtes) ;
- un appel sur le handle après la fin du rappel lève ;
- connectToProfile lancé pendant une transaction ne ferme la connexion qu'après le COMMIT ;
- remplacer :106-117 par un test qui documente la nouvelle règle.
Passe de mutations : remettre le drapeau global, retirer la révocation, fermer hors file : chacune rougit.

Critères d'acceptation

Aucune requête venue d'ailleurs ne s'exécute pendant une transaction (test rouge avant, vert après).
Le handle d'une transaction terminée lève.
Un changement de profil n'interrompt pas une transaction en cours.
npm test vert, les 8 sites de withTransaction inchangés fonctionnellement.

Complexité estimée

Medium.

Liens

#402 (revue, commentaire 20659), #396 (lectures du fournisseur d'onboarding), v0.10.1 (withTransaction).

Depends on

(aucune)

Critères d'acceptation (planification)

Handle de transaction : objet typé Database, qui exécute directement ; close() sur ce handle lève une erreur nommée ; tout appel après la fin du rappel lève une erreur nommée.
Section unique au changement de profil : connectToProfile, initializeNewProfileDb et closeDb sont chacun un seul runExclusive, qui enchaîne fermeture de l'ancien pool → (repair_migrations, prepare_profile_db pour connectToProfile) → Database.load → bascule de rawInstance/serializedInstance, via des helpers internes qui ne prennent jamais le verrou (closeUnlocked, loadUnlocked ou équivalent). prepare_profile_db n'est invoqué qu'après la résolution du close() de l'ancienne connexion.
Génération de connexion : chaque ouverture incrémente un numéro ; le proxy de getDb() et le handle de transaction le vérifient au moment où l'opération sort de la file ; une opération mise en file pour une génération fermée lève « connection closed by profile switch » (erreur nommée), au lieu de tourner sur la nouvelle base ou sur un pool fermé.
Alerte de blocage : une opération qui attend dans la file depuis plus de 15 s écrit un logWarn (une fois par opération) qui nomme ce qui tient le verrou (transaction, requête ou ouverture de profil) et depuis quand. logService n'écrit qu'en mémoire et en sessionStorage (vérifié), donc l'appel est sûr pendant que le verrou est tenu. Un rattrapage de plus de 15 s produit donc une alerte « ouverture de profil » : attendu, à dire dans le commentaire du code.
Tests dans db.test.ts (Database factice existant, L48-64) : lecture lancée pendant le rappel terminée après le COMMIT ; handle utilisé après le rappel → erreur ; close() sur le handle → erreur ; deux connectToProfile concurrents → un seul pool ouvert, celui du dernier fichier demandé ; withTransaction mis en file derrière connectToProfile → erreur de génération (pas d'exécution sur la nouvelle base) ; prepare_profile_db invoqué après le close() de l'ancienne connexion ; getDb() appelé dans un rappel → alerte écrite après le délai (minuteurs factices de vitest). Remplacer :106-117.
Passe de mutations, chacune rouge : remettre le drapeau global ; retirer la révocation ; retirer la vérification de génération ; fermer hors section ; retirer l'alerte.

Review caveats

🔴 (résolu) Verrou strict = blocage silencieux possible → alerte + exec obligatoire.
🔴 (résolu) Fermeture/chargement imbriqués et requêtes en file pour l'ancien profil → section unique + génération.
🟡 Pendant un rattrapage long, toute la file attend (choix assumé : rien d'utile ne peut tourner pendant la bascule).
🟢 Les tests de services mockent db.ts (withTransaction: fn => fn(await getDb())) : seul db.test.ts voit le verrou.

Décisions prises en planification

Verrou strict + fermeture par la file (Max, 2026-10-10, /analyze).
Alerte Journal à 15 s + exec obligatoire ; pas de scan des sources (Max, 2026-10-10).
Une section exclusive fermer → préparer → charger + génération de connexion (Max, 2026-10-10).
Sections de docs/architecture.md fixées pour toute la milestone : ### Accès DB et transactions (sous la section Base de données) et ## Tests (vitest sans jsdom) (juste avant CI/CD). La première issue de la pile qui en a besoin crée la section ; les suivantes y ajoutent.
Premier maillon de la pile #408 → #409 → #389 → #395 ; la milestone s'exécute après planned-2026-10-10-release-deps.

Spec source

spec-plan-fiabilite-tests.md (local, ignoré par git)

> **Planification du 2026-10-10** (`/plan-run`, milestone `planned-2026-10-10-fiabilite-tests`) : les sections ci-dessous, à partir de « Depends on », **priment** sur le texte d'analyse en cas d'écart. > Analyse `/analyze` du 2026-10-10, lue sur `main` `87920ec`. Corps d'origine archivé en commentaire. ## Contexte Revue de #402 (commentaire 20659, point 1). Pendant le rappel d'un `withTransaction`, le drapeau global `inTransaction` de `src/services/db.ts` (L30-35) fait passer **tout** `execute`/`select` hors du verrou FIFO (`serialize`, L54-72), d'où qu'il vienne. Une requête lancée ailleurs pendant une transaction s'exécute donc en parallèle d'elle : deux opérations en vol, ce que le commentaire de tête de `db.ts` donne comme cause du « database is locked » de la 0.10.0. `onboardingService.ts:128-131` le reconnaît déjà en commentaire. ### Vérifié en analyse - **Aucun rappel n'appelle `getDb()`**, directement ou par une fonction appelée. Les 8 sites passent le handle reçu : `balance.service.ts:710`, `:1509`, `:1597` ; `onboardingService.ts:144` ; `categoryMigrationService.ts:187` ; `categorizationService.ts:227` ; `dataExportService.ts:427` (`runRestore`, avec `restoreCategories`, `restoreSuppliers`, `restoreKeywords`, `restoreImportTemplates`, `restoreImportSources`, `attachTransactions`, qui prennent tous `db` en paramètre). Un verrou strict n'interbloque donc aucun chemin actuel. - **Piège latent** : `findOrCreateSecurity` (`balance.service.ts:1006`) fait `const db = exec ?? (await getDb())`. Un futur appel sans `exec` depuis un rappel interbloquerait sous la nouvelle règle. - **Défaut voisin, inclus** : `connectToProfile` (L163-167), `initializeNewProfileDb` (L195-204) et `closeDb` (L207-212) ferment `rawInstance` directement, sans attendre la file ni une transaction en cours. Un changement de profil (`ProfileContext.tsx:97-98`, `:179-180`) peut fermer la connexion au milieu d'une transaction. `initializeNewProfileDb` exécute aussi son SQL hors verrou. `lockTail` n'est jamais réinitialisé. - Le passage direct est voulu et épinglé par `db.test.ts:106-117` (« lets a nested getDb() call run directly inside a transaction — no deadlock »), à remplacer. ## Décisions (Max, 2026-10-10) - **D1 — Verrou strict.** Le rappel reçoit un handle propre à la transaction qui exécute directement ; tout autre appel (y compris `getDb()` depuis un rappel) attend derrière le verrou. Le drapeau global disparaît. - **D2 — Handle invalidé à la fin du rappel.** Après le `finally`, un appel sur ce handle lève une erreur nommée, au lieu de passer hors verrou depuis une promesse orpheline. - **D3 — Fermeture par la file.** `connectToProfile`, `initializeNewProfileDb` et `closeDb` ferment et rouvrent la connexion via `runExclusive`, donc après les opérations déjà en file et après une transaction en cours. ## Travail à faire - [ ] `db.ts` : supprimer `inTransaction` ; `withTransaction` crée un proxy de transaction (exécution directe sur `rawInstance` capturé) passé au rappel, révoqué dans le `finally`. - [ ] `db.ts` : `connectToProfile`, `initializeNewProfileDb`, `closeDb` passent leur fermeture/ouverture (et le SQL d'initialisation) par `runExclusive`. - [ ] Mettre à jour le commentaire de tête et ceux de L30-35 et L50-52 : la règle d'interblocage devient « ne jamais appeler `getDb()` depuis un rappel, utiliser le handle reçu ». - [ ] `balance.service.ts:1006` : rendre `exec` **obligatoire** sur `findOrCreateSecurity` (plus de `exec ?? getDb()`), appelants mis à jour. - [ ] Retirer le commentaire devenu faux de `onboardingService.ts:128-131`. - [ ] `docs/architecture.md`, section `### Accès DB et transactions` (à créer) : la règle du verrou, du handle de transaction, de la génération et de l'alerte. Aucun ADR ne porte `withTransaction` (vérifié) : pas d'ADR. - Pas d'entrée CHANGELOG (robustesse interne, aucun comportement visible attendu). ## Fichiers concernés - `src/services/db.ts`, `src/services/db.test.ts` - `src/services/balance.service.ts` (L1006) - `src/services/onboardingService.ts` (commentaire L128-131) - `docs/architecture.md` ## Surface de test - `db.test.ts` a un `Database` factice (L48-64, 2 ms par appel, journal début/fin), réutilisable. Les tests de services mockent `db.ts` (`withTransaction: fn => fn(await getDb())`) et ne voient pas le verrou : ne pas s'y fier. - À ajouter dans `db.test.ts` : - une lecture `getDb().select` lancée **pendant** le rappel (après `BEGIN`) se termine après le `COMMIT` (rouge aujourd'hui) ; - le handle du rappel exécute sans attendre (pas d'interblocage sur ses propres requêtes) ; - un appel sur le handle après la fin du rappel lève ; - `connectToProfile` lancé pendant une transaction ne ferme la connexion qu'après le `COMMIT` ; - remplacer `:106-117` par un test qui documente la nouvelle règle. - Passe de mutations : remettre le drapeau global, retirer la révocation, fermer hors file : chacune rougit. ## Critères d'acceptation - [ ] Aucune requête venue d'ailleurs ne s'exécute pendant une transaction (test rouge avant, vert après). - [ ] Le handle d'une transaction terminée lève. - [ ] Un changement de profil n'interrompt pas une transaction en cours. - [ ] `npm test` vert, les 8 sites de `withTransaction` inchangés fonctionnellement. ## Complexité estimée Medium. ## Liens #402 (revue, commentaire 20659), #396 (lectures du fournisseur d'onboarding), v0.10.1 (`withTransaction`). --- ## Depends on (aucune) ## Critères d'acceptation (planification) - [ ] **Handle de transaction** : objet typé `Database`, qui exécute directement ; `close()` sur ce handle lève une erreur nommée ; tout appel après la fin du rappel lève une erreur nommée. - [ ] **Section unique au changement de profil** : `connectToProfile`, `initializeNewProfileDb` et `closeDb` sont chacun **un seul** `runExclusive`, qui enchaîne fermeture de l'ancien pool → (`repair_migrations`, `prepare_profile_db` pour `connectToProfile`) → `Database.load` → bascule de `rawInstance`/`serializedInstance`, via des helpers internes qui ne prennent jamais le verrou (`closeUnlocked`, `loadUnlocked` ou équivalent). `prepare_profile_db` n'est invoqué qu'après la résolution du `close()` de l'ancienne connexion. - [ ] **Génération de connexion** : chaque ouverture incrémente un numéro ; le proxy de `getDb()` et le handle de transaction le vérifient au moment où l'opération sort de la file ; une opération mise en file pour une génération fermée lève « connection closed by profile switch » (erreur nommée), au lieu de tourner sur la nouvelle base ou sur un pool fermé. - [ ] **Alerte de blocage** : une opération qui attend dans la file depuis plus de 15 s écrit un `logWarn` (une fois par opération) qui nomme ce qui tient le verrou (transaction, requête ou ouverture de profil) et depuis quand. `logService` n'écrit qu'en mémoire et en `sessionStorage` (vérifié), donc l'appel est sûr pendant que le verrou est tenu. Un rattrapage de plus de 15 s produit donc une alerte « ouverture de profil » : attendu, à dire dans le commentaire du code. - [ ] Tests dans `db.test.ts` (Database factice existant, L48-64) : lecture lancée pendant le rappel terminée après le `COMMIT` ; handle utilisé après le rappel → erreur ; `close()` sur le handle → erreur ; deux `connectToProfile` concurrents → un seul pool ouvert, celui du dernier fichier demandé ; `withTransaction` mis en file derrière `connectToProfile` → erreur de génération (pas d'exécution sur la nouvelle base) ; `prepare_profile_db` invoqué après le `close()` de l'ancienne connexion ; `getDb()` appelé dans un rappel → alerte écrite après le délai (minuteurs factices de vitest). Remplacer `:106-117`. - [ ] Passe de mutations, chacune rouge : remettre le drapeau global ; retirer la révocation ; retirer la vérification de génération ; fermer hors section ; retirer l'alerte. ## Review caveats - 🔴 (résolu) Verrou strict = blocage silencieux possible → alerte + `exec` obligatoire. - 🔴 (résolu) Fermeture/chargement imbriqués et requêtes en file pour l'ancien profil → section unique + génération. - 🟡 Pendant un rattrapage long, toute la file attend (choix assumé : rien d'utile ne peut tourner pendant la bascule). - 🟢 Les tests de services mockent `db.ts` (`withTransaction: fn => fn(await getDb())`) : seul `db.test.ts` voit le verrou. ## Décisions prises en planification - Verrou strict + fermeture par la file (Max, 2026-10-10, `/analyze`). - Alerte Journal à 15 s + `exec` obligatoire ; pas de scan des sources (Max, 2026-10-10). - Une section exclusive fermer → préparer → charger + génération de connexion (Max, 2026-10-10). - Sections de `docs/architecture.md` fixées pour toute la milestone : `### Accès DB et transactions` (sous la section Base de données) et `## Tests (vitest sans jsdom)` (juste avant CI/CD). La première issue de la pile qui en a besoin crée la section ; les suivantes y ajoutent. - Premier maillon de la pile #408 → #409 → #389 → #395 ; la milestone s'exécute après `planned-2026-10-10-release-deps`. ## Spec source `spec-plan-fiabilite-tests.md` (local, ignoré par git)

maximus added the

status:triage

type:refactor

source:human

labels 2026-10-09 01:12:03 +00:00