feat(server): arrêt propre sur SIGTERM et SIGINT, et une ligne de log par incident - #54
Merged
Merged
Conversation
Pose un gestionnaire unique sur SIGTERM et SIGINT, installé par app.run(). Le vide laissé jusqu'ici conduisait les projets à poser le leur depuis un module utilitaire, dont le process.exit() emportait le reste de l'arrêt — et empêchait tout exportateur de télémétrie de vider son tampon. L'arrêt se déroule dans cet ordre : readiness à 503 puis attente de config.shutdownDelay pour laisser un load balancer sortir l'instance, fermeture du serveur HTTP, config.onShutdown(), puis la base et le cache. config.shutdownTimeout plafonne l'ensemble, un second signal sort immédiatement. Rien n'est installé en environnement de test. config.onShutdown est le point d'accroche unique du projet : il s'exécute après la fermeture du serveur, donc plus aucune requête n'est en cours, et avant la base et le cache, qu'il peut encore vouloir utiliser. Un rejet est journalisé et l'arrêt se poursuit — un pool qui n'a pas pu se vider n'est pas une raison de perdre les connexions qu'on allait rendre. app.shutdown() est exportée pour un cron ou un script, qui n'a aucun signal à attendre mais dont le pool maintient le processus en vie. Côté db, dbs.close() et Db.close() rendent les pools via un closePool ajouté aux deux drivers. query() sur une base fermée lève au lieu de recréer le pool : une requête tardive rouvrirait ce qu'on vient de fermer. app.server et les nouveaux réglages sont enfin déclarés dans index.d.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Un incident produisait deux lignes : l'une portait le message et la pile, l'autre la méthode, le chemin, le corps et la réponse. Aucune n'était complète, et les deux se corrélaient par le trace_id. L'erreur est désormais attachée à la ligne « request » : le message devient l'erreur, et la pile l'accompagne. Une route rendue en HTML se journalise comme une route JSON — l'attachement se fait avant l'aiguillage sur isApi. Gardent une ligne à part les deux cas qui n'ont rien où se greffer : une erreur levée après l'envoi de la réponse, et une erreur hors requête (uncaughtException, commande CLI). La cause d'une 500 n'est plus perdue en production. Le corps de la réponse y est volontairement vidé, et le log enregistrait ce vide ; le message porte maintenant l'erreur, quoi que le client ait reçu. body, query, params et response sont journalisés en chaînes JSON plutôt qu'en objets imbriqués : un collecteur qui aplatit les champs imbriqués éclatait un document problème en response_status, response_title et response_type. query et params sont journalisés quel que soit le statut, quand ils ne sont pas vides — ce qui a été demandé fait partie de la lecture d'une ligne, et ni l'un ni l'autre ne pèse comme un corps. body et response restent réservés aux lignes en erreur. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…logs Le garde-fou d'arrêt était désarmé par son propre unref(). Une fois le serveur HTTP fermé, plus aucun handle ne maintenait la boucle : un arrêt bloqué la laissait se vider et sortait en 0, silencieusement, en se faisant passer pour un arrêt réussi. Le minuteur reste désormais référencé et est annulé dans un finally. La fixture « hang » ne voyait rien parce qu'elle laissait le serveur à l'écoute ; elle le ferme maintenant avant de bloquer, et le test échoue bien sans le correctif. Une 500 disparaissait entièrement sous LOG_REQUESTS=false. Depuis que l'erreur voyage sur la ligne « request », son écriture passait par shouldLog(), là où l'ancien logger.error était inconditionnel. Le réglage coupe le journal d'accès, pas le signalement d'un plantage. Db.close() pouvait ne jamais rendre la main : la connexion gardée d'un test restait empruntée, et pool.end() l'attendait. Elle est rendue avant le drain. getConnection() vérifie aussi closed, pour les helpers de transaction qui ne passent pas par query() ; et query() teste closed avant pool, sans quoi un pool nul menait à init(), qui rouvrait la base et remettait closed à false. cache.scan() et flush() déréférençaient un client que close() venait de nuller : le contrat est un miss, pas un TypeError. La disponibilité est revérifiée à chaque aller-retour. closing reste levé après close() pour faire taire le listener du client démonté, et init() l'abaisse. ShutdownTest ne dépend plus de l'ordre alphabétique des fichiers pour rendre une readiness propre. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deux sujets liés par la même finalité : rendre une application igo observable au moment où elle s'arrête, et diagnosticable quand elle échoue.
Arrêt propre
app.run()pose un gestionnaire unique surSIGTERMetSIGINT. Le vide laissé jusqu'ici conduisait les projets à poser le leur depuis un module utilitaire, dont leprocess.exit()emportait le reste de l'arrêt — et empêchait tout exportateur de télémétrie de vider son tampon. C'est ce qui bloque aujourd'hui la mise en place d'OpenTelemetry sur LADOM.Déroulé :
config.shutdownDelay— le temps qu'un load balancer sorte l'instanceconfig.onShutdown()— le point d'accroche du projetconfig.shutdownTimeout(10 s) plafonne l'ensemble, un second signal sort immédiatement. Rien n'est installé en environnement de test.config.onShutdownest le point d'accroche unique : il s'exécute après la fermeture du serveur, donc plus aucune requête n'est en cours, et avant la base et le cache, qu'il peut encore vouloir utiliser. Un rejet est journalisé et l'arrêt se poursuit — un pool qui n'a pas pu se vider n'est pas une raison de perdre les connexions qu'on allait rendre.app.shutdown()est exportée pour un cron ou un script, qui n'a aucun signal à attendre mais dont le pool maintient le processus en vie.Côté db :
dbs.close()etDb.close()rendent les pools via unclosePoolajouté aux deux drivers.query()sur une base fermée lève au lieu de recréer le pool.Une ligne de log par incident
Un incident produisait deux lignes : l'une portait le message et la pile, l'autre la méthode, le chemin, le corps et la réponse. Aucune n'était complète.
L'erreur est désormais attachée à la ligne
request: le message devient l'erreur, et la pile l'accompagne. Vaut aussi pour les routes rendues en HTML — l'attachement se fait avant l'aiguillage surisApi.La cause d'une 500 n'est plus perdue en production, où le corps de la réponse est volontairement vidé : le log enregistrait ce vide, le message porte maintenant l'erreur.
body,query,paramsetresponsesont journalisés en chaînes JSON plutôt qu'en objets imbriqués — un collecteur qui aplatit les champs imbriqués éclatait un document problème enresponse_status,response_titleetresponse_type.queryetparamssont présents quel que soit le statut quand ils ne sont pas vides ;bodyetresponserestent réservés aux erreurs, ce sont eux le volume.Revue
Une revue a relevé six défauts, tous corrigés dans
df4158f. Deux méritent d'être signalés :unref(): une fois le serveur fermé, un arrêt bloqué sortait en 0, silencieusement. La fixturehangne le voyait pas car elle laissait le serveur à l'écoute.LOG_REQUESTS=false, l'erreur passant désormais parshouldLog().Les deux ont un test vérifié comme échouant sans son correctif.
Vérification
808 tests sur les quatre paquets, eslint et le build de la doc passent. La partie arrêt reste à éprouver sur un projet réel — LADOM est le cas d'épreuve, avec son gestionnaire concurrent à faire cohabiter.
Reste à faire
6.4.0paraît plus juste que6.3.2, la PR ajoutantconfig.onShutdown,app.shutdown()et trois réglages.🤖 Generated with Claude Code