Compare commits

..

1 Commits

Author SHA1 Message Date
MechaCat02
156d9e427d feat: handle SIGTERM for graceful container stops (0.35.0)
`docker compose stop` and Kubernetes / Podman / systemd all send
SIGTERM first; SIGINT is for interactive shells. Without a SIGTERM
listener the container's stop-grace period elapses with the API still
running, then SIGKILL skips the daemon shutdown path and leaks
Chromium until the OS reaps the parent. Replace the bare
`tokio::signal::ctrl_c()` with a select over ctrl_c and
SignalKind::terminate() so the daemon.shutdown().await path runs in
both cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-28 18:59:33 +02:00
4 changed files with 32 additions and 138 deletions

View File

@@ -17,10 +17,7 @@ async fn main() -> anyhow::Result<()> {
tracing::info!(%addr, "mangalord listening");
let listener = tokio::net::TcpListener::bind(addr).await?;
axum::serve(listener, router)
.with_graceful_shutdown(async {
let _ = tokio::signal::ctrl_c().await;
tracing::info!("ctrl-c received; shutting down");
})
.with_graceful_shutdown(shutdown_signal())
.await?;
// Drain background tasks (crawler daemon) before exiting so Chromium
@@ -30,3 +27,33 @@ async fn main() -> anyhow::Result<()> {
}
Ok(())
}
/// Wait for either Ctrl-C (interactive shell) or SIGTERM (Docker /
/// Kubernetes / Podman / systemd stop) and log which arrived. Without
/// the SIGTERM branch, `docker compose stop` runs out its grace period
/// and skips straight to SIGKILL — the daemon never gets the
/// `daemon.shutdown().await` path, leaking Chromium.
async fn shutdown_signal() {
use tokio::signal::unix::{signal, SignalKind};
let mut sigterm = match signal(SignalKind::terminate()) {
Ok(s) => s,
Err(e) => {
// SignalKind::terminate() is supported on every Unix the
// tokio runtime runs on; if registration fails we still
// honour Ctrl-C so the process is at least
// interactive-shutdownable.
tracing::warn!(error = %e, "could not install SIGTERM handler; falling back to ctrl_c only");
let _ = tokio::signal::ctrl_c().await;
tracing::info!("ctrl-c received; shutting down");
return;
}
};
tokio::select! {
_ = tokio::signal::ctrl_c() => {
tracing::info!("ctrl-c received; shutting down");
}
_ = sigterm.recv() => {
tracing::info!("SIGTERM received; shutting down");
}
}
}

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest';
import { ApiError, request, setOn401Hook } from './client';
import { ApiError, request } from './client';
import { getManga } from './mangas';
describe('request error envelope parsing', () => {
@@ -73,88 +73,3 @@ describe('request error envelope parsing', () => {
expect(err.code).toBe('http_error');
});
});
describe('on401 hook', () => {
let fetchSpy: MockInstance<typeof globalThis.fetch>;
beforeEach(() => {
fetchSpy = vi.spyOn(globalThis, 'fetch');
});
afterEach(() => {
vi.restoreAllMocks();
// Critical: reset the module-level hook between tests so a
// hook installed by one test doesn't leak into the next.
setOn401Hook(null);
});
it('invokes the hook exactly once on a 401 response and re-throws', async () => {
const hook = vi.fn();
setOn401Hook(hook);
fetchSpy.mockResolvedValueOnce(
new Response(
JSON.stringify({ error: { code: 'unauthenticated', message: 'no auth' } }),
{ status: 401, headers: { 'content-type': 'application/json' } }
)
);
await expect(getManga('x')).rejects.toMatchObject({
status: 401,
code: 'unauthenticated'
});
expect(hook).toHaveBeenCalledTimes(1);
});
it('does not invoke the hook on non-401 errors', async () => {
const hook = vi.fn();
setOn401Hook(hook);
fetchSpy.mockResolvedValueOnce(
new Response(
JSON.stringify({ error: { code: 'not_found', message: 'no' } }),
{ status: 404, headers: { 'content-type': 'application/json' } }
)
);
await expect(getManga('x')).rejects.toMatchObject({ status: 404 });
expect(hook).not.toHaveBeenCalled();
});
it('does not invoke the hook on successful responses', async () => {
const hook = vi.fn();
setOn401Hook(hook);
fetchSpy.mockResolvedValueOnce(
new Response(
JSON.stringify({
id: 'm1',
title: 't',
status: 'ongoing',
alt_titles: [],
description: null,
cover_image_path: null,
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z',
authors: [],
genres: [],
tags: []
}),
{ status: 200, headers: { 'content-type': 'application/json' } }
)
);
await getManga('m1');
expect(hook).not.toHaveBeenCalled();
});
it('swallows hook exceptions so the original ApiError still propagates', async () => {
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
setOn401Hook(() => {
throw new Error('hook boom');
});
fetchSpy.mockResolvedValueOnce(
new Response(
JSON.stringify({ error: { code: 'unauthenticated', message: 'x' } }),
{ status: 401, headers: { 'content-type': 'application/json' } }
)
);
await expect(getManga('x')).rejects.toMatchObject({ status: 401 });
// The original ApiError won — the hook's panic was logged but
// didn't replace the API error.
expect(consoleSpy).toHaveBeenCalled();
});
});

View File

@@ -25,21 +25,6 @@ export class ApiError extends Error {
type ErrorEnvelope = { error?: { code?: unknown; message?: unknown } };
/**
* Optional hook fired the first moment `request()` observes a 401 on
* any endpoint. Used by the session store to clear the cached user
* when the server reports the session is no longer valid (expired
* cookie, rotated server-side, password changed on another device).
*
* Set to `null` (or `undefined`) to disable. Tests that don't want
* the side effect should leave it unset.
*/
let on401Hook: (() => void) | null = null;
export function setOn401Hook(handler: (() => void) | null): void {
on401Hook = handler;
}
export async function request<T>(path: string, init?: RequestInit): Promise<T> {
// Forward credentials (session cookie) explicitly so cross-origin
// deployments — those configured via CORS_ALLOWED_ORIGINS — keep
@@ -69,16 +54,6 @@ export async function request<T>(path: string, init?: RequestInit): Promise<T> {
} catch {
// Body wasn't parseable; keep the http_error fallback.
}
if (res.status === 401 && on401Hook) {
// Fire before throwing so the session store updates even
// if the caller swallows the ApiError (e.g. the *OrEmpty
// wrappers used by guest-rendering pages).
try {
on401Hook();
} catch (e) {
console.error('on401 hook threw:', e);
}
}
throw new ApiError(res.status, code, message);
}
// Any empty body (not just 204) returns undefined — the manga-add

View File

@@ -3,17 +3,7 @@
// Only mutated client-side (onMount / form submits) so the module-level
// instance can't leak across SSR requests — SSR always renders the
// `loaded === false` state, and the client refreshes after hydration.
//
// IMPORTANT: do not call any `api/*` helper from `+page.server.ts` /
// `+layout.server.ts`. The `setOn401Hook` below is registered at
// module load (gated on `browser`, so it only fires in the client
// bundle), so a 401 from a server-side fetch would mutate this
// module-level `session.user` across SvelteKit requests — a real
// cross-request state leak. The `if (browser)` guard makes that
// failure mode mechanical rather than convention-based.
import { browser } from '$app/environment';
import { setOn401Hook } from './api/client';
import { me, type User } from './api/auth';
class SessionStore {
@@ -41,16 +31,3 @@ class SessionStore {
}
export const session = new SessionStore();
// When any backend call returns 401, drop the cached user. Before this
// hook, the `*OrEmpty` wrappers silently returned empty pages on 401
// — so a mid-session expiry left the UI rendering as "logged in but
// no bookmarks/collections/etc." until the user manually reloaded.
// With the hook the session.user reactive store flips to null on the
// first 401, so the layout re-renders the login affordance.
//
// Gated on `browser` so it's only installed in the client bundle.
// See the module-level comment above for the SSR rationale.
if (browser) {
setOn401Hook(() => session.setUser(null));
}