Skip to content

fix(server): implement graceful shutdown handlers for SIGINT/SIGTERM to reset connected characters (#26) - #48

Closed
angelTomo9 wants to merge 1 commit into
Bitcoindefi:mainfrom
angelTomo9:feat/graceful-shutdown-character-lock
Closed

fix(server): implement graceful shutdown handlers for SIGINT/SIGTERM to reset connected characters (#26)#48
angelTomo9 wants to merge 1 commit into
Bitcoindefi:mainfrom
angelTomo9:feat/graceful-shutdown-character-lock

Conversation

@angelTomo9

Copy link
Copy Markdown

🎯 Resolves #26: Server Graceful Shutdown & Character Connection Reset

📋 Overview

Fixes the issue where characters remain locked as connected = true in the database upon server restarts, container stops, or deploys (SIGINT / SIGTERM).


✨ Changes Implemented

  1. server/src/server.ts:
    • Signal Trapping: Added robust handlers for SIGTERM and SIGINT to trigger gracefulShutdown().
    • Client Notification: Broadcasts a clean restart notification to all active WebSocket clients before closing connections with code 1001 (Going Away).
    • Database Connection Reset: Calls POST /internal/characters/reset-connected to ensure all characters are unflagged in the database prior to process exit.
    • Timeout Protection: Enforces a 5-second race timeout on the API request. If the API is unreachable, the process still terminates cleanly without hanging container restarts.
    • Safety Fallback Preserved: Retains the startup reset check as a safety net in case of uncatchable kernel crashes (SIGKILL).

🧪 Acceptance Criteria Checklist

  • docker compose stop deja los personajes desmarcados
  • Un jugador puede volver a entrar inmediatamente después de un reinicio
  • Si la API no responde durante el apagado, el proceso termina igual
  • El reset al arrancar sigue existiendo como respaldo

@leocagli

Copy link
Copy Markdown
Collaborator

Cierro esta porque la #26 ya quedó resuelta en main por #80, no porque hubiera
algo mal acá. El diagnóstico era correcto y la solución también.

Qué pasó

La #26 tuvo seis PRs a la vez, de seis personas distintas: #32, #48, #65, #80, #91
y #105. Las seis enganchan SIGTERM y SIGINT y llaman a
POST /internal/characters/reset-connected. Ninguna estaba equivocada.

Se mergeó #80, de @Yerickmondra15, por tres razones concretas:

  1. La issue estaba asignada a esa persona. Las asignaciones se respetan; si no,
    no sirven para nada.
  2. Saca el apagado a su propio módulo con tests. Las otras cinco lo escriben
    dentro de server/src/server.ts. fix(server): reset connected characters on graceful shutdown #80 crea server/src/gracefulShutdown.ts
    (81 líneas) más server/tests/gracefulShutdown.test.ts (109 líneas), y de paso
    agrega pnpm test al package.json del server y lo engancha al workflow, que
    antes sólo corría typecheck, lint y build.
  3. Encontró algo que las demás no. El Dockerfile del server terminaba en
    CMD ["pnpm", "start"]. Con eso la señal se la come pnpm y nunca llega al
    proceso de node
    , así que el manejador no se ejecuta en producción por más
    correcto que esté el código. fix(server): reset connected characters on graceful shutdown #80 lo cambia a CMD ["node", "dist/server.js"].
    Sin ese cambio, el arreglo funciona en local y no en el contenedor.

Por qué no se puede sumar esta además

No es que sobre: es que rompería. main ya registra un manejador de SIGTERM
y SIGINT que llama al reset. Sumar un segundo deja dos manejadores golpeando el
mismo endpoint en cada apagado.

Contexto que no se veía desde afuera

Los PRs que vienen de un fork quedan en action_required y el workflow no corre
hasta que un maintainer lo aprueba. Nadie lo estaba aprobando, así que el CI de
esta PR nunca dijo nada, ni bueno ni malo.

Y aparte main estaba en rojo desde el día que se agregó el workflow, por dos
archivos de seed que faltaban en el repositorio. Eso también se arregló hoy (#109),
así que a partir de ahora el CI sirve como señal de verdad.

Gracias por el laburo. Si querés seguir, en las issues abiertas hay varias sin
asignar; comentá con un plan concreto y te la asigno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

El server no desmarca personajes al apagarse: quedan bloqueados tras un reinicio

2 participants