Skip to content

Reset connected characters on shutdown - #65

Closed
telemarkdigital-publisher wants to merge 1 commit into
Bitcoindefi:mainfrom
telemarkdigital-publisher:telemark-graceful-shutdown-26
Closed

Reset connected characters on shutdown#65
telemarkdigital-publisher wants to merge 1 commit into
Bitcoindefi:mainfrom
telemarkdigital-publisher:telemark-graceful-shutdown-26

Conversation

@telemarkdigital-publisher

Copy link
Copy Markdown

Summary

  • add SIGINT/SIGTERM graceful shutdown handling in the game server
  • warn connected clients before shutdown and flush the outbound queue
  • call the existing /internal/characters/reset-connected endpoint with an 8s timeout so shutdown cannot hang indefinitely
  • keep the existing startup reset as a fallback for abrupt crashes

Closes #26

Verification

  • server tsc --noEmit
  • server eslint ./src/**/*.ts
  • git diff --check

@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

3 participants