mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(i18n): reconcile the stored locale from the request, not from a lost PUT
`User.locale` is the only signal a cookie-blind path has. The nightly briefing warm, the Coach nudge, the reminder senders and the native aggregate all resolve from that column; the screen in front of the user resolves from the `healthlog-locale` cookie first, then the column, then Accept-Language. So a browser can read the whole app in German while the column says English, and while it does, nothing on screen looks wrong: the divergence surfaces as one generated paragraph in the wrong language inside an otherwise correct page. Keeping the two in step was a fire-and-forget PUT from a mount effect. A 401 on the paint before sign-in, an offline blip, a tab closed mid-flight — each loses the write, and the next mount repeats the same unchecked attempt rather than noticing the column is still wrong, so a stale column can stand for the life of the account. `getSession` now corrects the column when the request carries a valid locale cookie the column disagrees with. It is ordered by the request pipeline, needs nothing from the client, and is self-limiting: once the two agree the condition is false. Only a cookie counts, and a cookie is only ever written alongside a committed UI locale, so Accept-Language never stamps a guess onto a column that generated prose depends on. The mount PUT stays for the server-set cookie it gets in reply, which Safari's 7-day cap on script-written cookies makes worth having; it is no longer the mechanism. The obvious suspect was a first-paint race between two unordered writes. It is not that: the provider takes the locale the server already resolved and nothing on the mount path changes it, so the effect fires once with the right value. `locale-first-paint-single-write` pins both properties so a future mount-time flip has to face them.
This commit is contained in:
1 parent
86f826351f
commit
1850e2f564
5 files changed
+409
-18
No files matched your search
@@ -0,0 +1,171 @@
|
||||
/**
|
||||
* `getSession` keeps `User.locale` in step with the language the browser is
|
||||
* actually reading.
|
||||
*
|
||||
* The column is the only signal a cookie-blind path has — the nightly
|
||||
* briefing warm, the Coach nudge, the reminder senders, the native dashboard
|
||||
* aggregate. The screen in front of the user resolves from the
|
||||
* `healthlog-locale` cookie first. When those two disagree nothing looks
|
||||
* wrong until a job writes a sentence, and then one paragraph of an
|
||||
* otherwise correct page comes back in the wrong language.
|
||||
*
|
||||
* Closing that used to be a fire-and-forget PUT from a mount effect, so a
|
||||
* lost request left the divergence standing for the life of the account.
|
||||
* These pin the server-side reconcile that replaced it as the mechanism.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const cookieState = vi.hoisted(() => ({
|
||||
sessionId: undefined as string | undefined,
|
||||
locale: undefined as string | undefined,
|
||||
set: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
}));
|
||||
|
||||
const store = vi.hoisted(() => ({
|
||||
row: null as Record<string, unknown> | null,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
session: {
|
||||
findUnique: vi.fn(async () => store.row),
|
||||
update: vi.fn(async () => ({})),
|
||||
deleteMany: vi.fn(async () => ({})),
|
||||
},
|
||||
user: { update: vi.fn(async () => ({})) },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/context", () => ({ getEvent: () => undefined }));
|
||||
|
||||
vi.mock("@/lib/auth/hmac", () => ({
|
||||
hashToken: (raw: string) => `hash:${raw}`,
|
||||
}));
|
||||
|
||||
vi.mock("next/headers", () => ({
|
||||
cookies: vi.fn(async () => ({
|
||||
get: (name: string) => {
|
||||
if (name === "healthlog_session" && cookieState.sessionId) {
|
||||
return { value: cookieState.sessionId };
|
||||
}
|
||||
if (name === "healthlog-locale" && cookieState.locale !== undefined) {
|
||||
return { value: cookieState.locale };
|
||||
}
|
||||
return undefined;
|
||||
},
|
||||
set: cookieState.set,
|
||||
delete: cookieState.delete,
|
||||
})),
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "../session";
|
||||
|
||||
const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000;
|
||||
|
||||
function seedSession(userLocale: string | null) {
|
||||
cookieState.sessionId = "hls_raw-secret";
|
||||
store.row = {
|
||||
id: "sess-1",
|
||||
userId: "user-1",
|
||||
tokenHash: "hash:hls_raw-secret",
|
||||
// Far enough out that the sliding-expiry refresh stays quiet.
|
||||
expiresAt: new Date(Date.now() + THIRTY_DAYS_MS),
|
||||
lastActiveAt: new Date(),
|
||||
actingAsUserId: null,
|
||||
recordEpoch: 0,
|
||||
user: { id: "user-1", locale: userLocale },
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
cookieState.locale = undefined;
|
||||
store.row = null;
|
||||
});
|
||||
|
||||
describe("getSession — locale column reconcile", () => {
|
||||
it("writes the column when the browser reads a different language", async () => {
|
||||
// The row the production instance actually held: a German reader whose
|
||||
// column said English, so every generated sentence came back English.
|
||||
seedSession("en");
|
||||
cookieState.locale = "de";
|
||||
|
||||
const result = await getSession();
|
||||
|
||||
expect(prisma.user.update).toHaveBeenCalledWith({
|
||||
where: { id: "user-1" },
|
||||
data: { locale: "de" },
|
||||
});
|
||||
// The same request must not answer with the value just replaced.
|
||||
expect(result?.user.locale).toBe("de");
|
||||
});
|
||||
|
||||
it("backfills a column that was never written at all", async () => {
|
||||
// The account that never opened the language picker: the cookie is the
|
||||
// client's own mount-time write of the Accept-Language paint.
|
||||
seedSession(null);
|
||||
cookieState.locale = "de";
|
||||
|
||||
await getSession();
|
||||
|
||||
expect(prisma.user.update).toHaveBeenCalledWith({
|
||||
where: { id: "user-1" },
|
||||
data: { locale: "de" },
|
||||
});
|
||||
});
|
||||
|
||||
it("writes nothing once the two agree", async () => {
|
||||
seedSession("de");
|
||||
cookieState.locale = "de";
|
||||
|
||||
await getSession();
|
||||
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores a cookie value that is not a shipped locale", async () => {
|
||||
// The cookie is not HttpOnly, so anything can be in it. A value the
|
||||
// resolver would later have to defend against never reaches the column.
|
||||
seedSession("de");
|
||||
cookieState.locale = "tlh";
|
||||
|
||||
const result = await getSession();
|
||||
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
expect(result?.user.locale).toBe("de");
|
||||
});
|
||||
|
||||
it("re-emits the cookie instead of writing when the cookie is gone", async () => {
|
||||
// Safari ITP expired the script-written copy; the column is the
|
||||
// survivor, so it is the one that seeds the cookie, not the reverse.
|
||||
seedSession("de");
|
||||
cookieState.locale = undefined;
|
||||
|
||||
await getSession();
|
||||
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
expect(cookieState.set).toHaveBeenCalledWith(
|
||||
"healthlog-locale",
|
||||
"de",
|
||||
expect.objectContaining({ httpOnly: false }),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not block the request on the write", async () => {
|
||||
// Fire-and-forget like the `lastActiveAt` stamp: a failing update must
|
||||
// not turn locale drift into a failed session resolution.
|
||||
seedSession("en");
|
||||
cookieState.locale = "de";
|
||||
vi.mocked(prisma.user.update).mockRejectedValueOnce(
|
||||
new Error("db down") as never,
|
||||
);
|
||||
|
||||
await expect(getSession()).resolves.not.toBeNull();
|
||||
});
|
||||
});
|
||||
+55
-11
@@ -325,26 +325,70 @@ export async function getSession(): Promise<{
|
||||
.catch(() => {});
|
||||
}
|
||||
|
||||
// Locale-cookie refresh: Safari's ITP expires the script-written
|
||||
// `healthlog-locale` cookie after 7 days, so a returning user's first
|
||||
// paint fell back to the browser language once a week. When the user
|
||||
// has a persisted locale but the cookie is gone, re-emit it here —
|
||||
// Set-Cookie is exempt from the ITP cap. Absent-only on purpose: a
|
||||
// locale switch writes the cookie client-side before the PUT persists
|
||||
// the column, and overwriting a *differing* cookie here would revert
|
||||
// that fresh choice mid-flight. Fail-soft: in a server-component
|
||||
// render the cookie store is read-only and `set` throws — the next
|
||||
// route-handler request re-attempts.
|
||||
const rawLocaleCookie = cookieStore.get(LOCALE_COOKIE)?.value;
|
||||
const cookieLocale =
|
||||
rawLocaleCookie && (locales as readonly string[]).includes(rawLocaleCookie)
|
||||
? (rawLocaleCookie as Locale)
|
||||
: null;
|
||||
|
||||
if (
|
||||
session.user.locale &&
|
||||
(locales as readonly string[]).includes(session.user.locale) &&
|
||||
!cookieStore.get(LOCALE_COOKIE)
|
||||
!cookieLocale
|
||||
) {
|
||||
// Locale-cookie refresh: Safari's ITP expires the script-written
|
||||
// `healthlog-locale` cookie after 7 days, so a returning user's first
|
||||
// paint fell back to the browser language once a week. When the user
|
||||
// has a persisted locale but the cookie is gone, re-emit it here —
|
||||
// Set-Cookie is exempt from the ITP cap. Fail-soft: in a
|
||||
// server-component render the cookie store is read-only and `set`
|
||||
// throws — the next route-handler request re-attempts.
|
||||
try {
|
||||
setLocaleCookie(cookieStore, session.user.locale as Locale);
|
||||
} catch {
|
||||
// Read-only cookie context — skip; nothing depends on this write.
|
||||
}
|
||||
} else if (cookieLocale && cookieLocale !== session.user.locale) {
|
||||
// The column follows the language the browser is actually reading.
|
||||
//
|
||||
// `User.locale` is the ONLY signal every cookie-blind path has: the
|
||||
// nightly briefing warm, the Coach nudge, the reminder senders and the
|
||||
// native aggregate all resolve from the column alone, while the screen
|
||||
// in front of the user resolves from this cookie first. The two can
|
||||
// therefore disagree indefinitely, and the disagreement is invisible —
|
||||
// it surfaces as one generated paragraph in the wrong language inside
|
||||
// an otherwise correct page.
|
||||
//
|
||||
// Keeping them in step used to be the client's job: a fire-and-forget
|
||||
// `PUT /api/auth/me/locale` from a mount effect, best-effort by
|
||||
// construction. A 401 on the paint before sign-in, an offline blip, a
|
||||
// tab closed mid-flight — every one of those loses the write silently,
|
||||
// and the next mount repeats the same unreliable attempt rather than
|
||||
// noticing the column is still wrong. So the column stayed on a
|
||||
// language the user had left behind, for as long as the account existed.
|
||||
//
|
||||
// This arm closes it from the side that cannot miss. The cookie is only
|
||||
// ever written alongside a committed UI locale (client `persistLocale`,
|
||||
// or this function's own refresh above), so it is a statement about what
|
||||
// the person is reading, not a guess: Accept-Language never reaches the
|
||||
// column this way. Fire-and-forget like the `lastActiveAt` stamp, and
|
||||
// self-limiting — once the two agree the condition is false and no
|
||||
// further write is attempted.
|
||||
//
|
||||
// A second browser whose cookie still carries the older choice will move
|
||||
// the column back. That is the same last-writer-wins the client PUT
|
||||
// already had, and it is coherent: that browser is showing the older
|
||||
// language too, because the cookie outranks the column on its first
|
||||
// paint as well.
|
||||
void prisma.user
|
||||
.update({
|
||||
where: { id: session.user.id },
|
||||
data: { locale: cookieLocale },
|
||||
})
|
||||
.catch(() => {});
|
||||
// Keep THIS request coherent — a handler resolving the locale from the
|
||||
// returned row must not answer with the value we just replaced.
|
||||
session.user.locale = cookieLocale;
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* The UI language and the stored language are resolved from different
|
||||
* evidence, and only one of them is ever shown to the person reading.
|
||||
*
|
||||
* `resolveServerLocale` / `resolveInitialLocale` answer a REQUEST: the
|
||||
* `healthlog-locale` cookie first, then `User.locale`, then Accept-Language.
|
||||
* Two of those three rungs are invisible to anything without a request, so
|
||||
* a browser can render the whole app in German while the column says
|
||||
* English.
|
||||
*
|
||||
* Everything written outside a request reads the column alone —
|
||||
* `resolveJobLocale` for the nightly writers, `resolveLocaleForUser` for the
|
||||
* Coach tool executor and the MCP reads. So a diverged column does not show
|
||||
* up as a wrong menu or a wrong button. It shows up as one paragraph of
|
||||
* generated prose in the wrong language, sitting in a page that is otherwise
|
||||
* correct, which is exactly how it was reported.
|
||||
*
|
||||
* These are the two ways a browser gets a language the column never hears
|
||||
* about. They are the reason `getSession` reconciles the column from the
|
||||
* cookie: the client's fire-and-forget PUT is the only thing that ever
|
||||
* closed this gap, and a request that never arrives leaves the divergence
|
||||
* standing for as long as the account exists.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const mockCookies = vi.fn();
|
||||
const mockHeaders = vi.fn();
|
||||
vi.mock("next/headers", () => ({
|
||||
cookies: (...args: unknown[]) => mockCookies(...args),
|
||||
headers: (...args: unknown[]) => mockHeaders(...args),
|
||||
}));
|
||||
|
||||
const mockGetSessionUserLocale = vi.fn();
|
||||
vi.mock("@/lib/auth/session", () => ({
|
||||
getSessionUserLocale: (...args: unknown[]) =>
|
||||
mockGetSessionUserLocale(...args),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/app-settings", () => ({
|
||||
getOperatorDefaultLocale: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
import { resolveInitialLocale } from "@/lib/i18n/resolve-initial-locale";
|
||||
import { resolveJobLocale } from "@/lib/i18n/job-locale";
|
||||
|
||||
function cookieStoreWith(value: string | undefined) {
|
||||
return {
|
||||
get: (name: string) =>
|
||||
name === "healthlog-locale" && value !== undefined
|
||||
? { name, value }
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
function headersWith(acceptLanguage: string | null) {
|
||||
return {
|
||||
get: (name: string) =>
|
||||
name.toLowerCase() === "accept-language" ? acceptLanguage : null,
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockCookies.mockResolvedValue(cookieStoreWith(undefined));
|
||||
mockHeaders.mockResolvedValue(headersWith(null));
|
||||
mockGetSessionUserLocale.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
describe("a browser can read German while the column says English", () => {
|
||||
it("Accept-Language alone: German UI, English background prose", async () => {
|
||||
// The account has never opened the language picker, so the column is
|
||||
// NULL, and the browser asks for German.
|
||||
mockGetSessionUserLocale.mockResolvedValue(null);
|
||||
mockHeaders.mockResolvedValue(headersWith("de-DE,de;q=0.9,en;q=0.5"));
|
||||
|
||||
await expect(resolveInitialLocale()).resolves.toBe("de");
|
||||
await expect(resolveJobLocale(null)).resolves.toBe("en");
|
||||
});
|
||||
|
||||
it("cookie alone: German UI, English background prose", async () => {
|
||||
// The picker was used at some point, so the cookie carries German —
|
||||
// but the column was left holding the language of the paint that
|
||||
// preceded the choice.
|
||||
mockCookies.mockResolvedValue(cookieStoreWith("de"));
|
||||
mockGetSessionUserLocale.mockResolvedValue("en");
|
||||
|
||||
await expect(resolveInitialLocale()).resolves.toBe("de");
|
||||
await expect(resolveJobLocale("en")).resolves.toBe("en");
|
||||
});
|
||||
|
||||
it("the cookie outranks the column on every request that has one", async () => {
|
||||
// Which is why the divergence is invisible: nothing the user looks at
|
||||
// reads the column, so a wrong column produces no wrong screen until a
|
||||
// job writes a sentence with it.
|
||||
mockCookies.mockResolvedValue(cookieStoreWith("de"));
|
||||
mockGetSessionUserLocale.mockResolvedValue("en");
|
||||
await expect(resolveInitialLocale()).resolves.toBe("de");
|
||||
expect(mockGetSessionUserLocale).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,70 @@
|
||||
/**
|
||||
* Why the obvious suspect is not the cause.
|
||||
*
|
||||
* The tempting explanation for a column stuck on the wrong language is a
|
||||
* race: first paint renders the default, the mount effect persists THAT,
|
||||
* the client then flips to the real locale and persists again, and two
|
||||
* unordered `keepalive` PUTs land in the wrong order. It would explain the
|
||||
* symptom exactly, and it would mean the write needs ordering.
|
||||
*
|
||||
* It is not what happens. The provider takes the locale the SERVER already
|
||||
* resolved — cookie, then column, then Accept-Language — and nothing on the
|
||||
* mount path changes it afterwards, so the effect fires once, with the right
|
||||
* value, and there is no second write to race with. The write is lost a
|
||||
* simpler way: it is fire-and-forget, so a 401 on the paint before sign-in,
|
||||
* an offline blip or a tab closed mid-flight drops it, and the next mount
|
||||
* repeats the same unreliable attempt rather than noticing.
|
||||
*
|
||||
* These pin the two properties that rule the race out, so that a future
|
||||
* change which does introduce a mount-time flip has to face this file.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
import { I18nProvider, useTranslations } from "@/lib/i18n/context";
|
||||
import deMessages from "../../../../messages/de.json";
|
||||
|
||||
function Read({ k }: { k: string }) {
|
||||
const { t } = useTranslations();
|
||||
return <span>{t(k)}</span>;
|
||||
}
|
||||
|
||||
describe("first paint carries the resolved locale, not a default", () => {
|
||||
it("renders the server-resolved locale on the very first pass", () => {
|
||||
// If this ever rendered English first, the mount effect would persist
|
||||
// English before the flip, and the race above would be real.
|
||||
const html = renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="de" initialMessages={deMessages}>
|
||||
<Read k="common.save" />
|
||||
</I18nProvider>,
|
||||
);
|
||||
expect(html).toContain("Speichern");
|
||||
expect(html).not.toContain(">Save<");
|
||||
});
|
||||
|
||||
it("changes the active locale only through the switcher", () => {
|
||||
const src = readFileSync(
|
||||
join(process.cwd(), "src/lib/i18n/context.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
expect(src.length).toBeGreaterThan(0);
|
||||
|
||||
// Every `setActive` call in the file. One belongs to `setLocale` (the
|
||||
// explicit switch); the others are the bundle backfill, which must
|
||||
// carry the locale it was given and never a different one.
|
||||
const setActiveCalls = src.match(/setActive\(/g) ?? [];
|
||||
expect(setActiveCalls.length).toBeGreaterThan(0);
|
||||
|
||||
// The backfill keeps `prev` whenever the locale moved under it, so it
|
||||
// can only ever swap MESSAGES, never the locale.
|
||||
expect(src).toMatch(
|
||||
/prev\.locale === locale \? \{ locale, messages: loaded \} : prev/,
|
||||
);
|
||||
|
||||
// And the persistence effect keys on the locale alone, so one mount
|
||||
// with one locale means one write.
|
||||
expect(src).toMatch(/persistLocaleToServer\(locale\);\s*\}, \[locale\]\);/);
|
||||
});
|
||||
});
|
||||
@@ -109,13 +109,19 @@ function persistLocale(newLocale: Locale) {
|
||||
document.documentElement.lang = newLocale;
|
||||
}
|
||||
|
||||
// v1.25.0 — mirror the active locale onto `User.locale` so server-side
|
||||
// background work (the proactive Coach nudge, the Telegram test message)
|
||||
// renders in the user's language. The cookie/localStorage choice is
|
||||
// invisible to a cron that has no request context; without this write the
|
||||
// column stays null and those messages fall back to English. Fire-and-forget
|
||||
// and idempotent on the server: a 401 on a public page, an offline blip or a
|
||||
// no-op equal value all fail silently and never block the UI flip.
|
||||
// Mirror the active locale onto `User.locale` so server-side background work
|
||||
// (the proactive Coach nudge, the Telegram test message) renders in the
|
||||
// user's language. The cookie/localStorage choice is invisible to a cron that
|
||||
// has no request context.
|
||||
//
|
||||
// This is no longer what the column depends on. Being fire-and-forget, it
|
||||
// loses the write on a 401, an offline blip or a tab closed mid-flight, and
|
||||
// nothing retried — the column then held a language the user had left behind
|
||||
// for good. `getSession` reconciles the column from the locale cookie on the
|
||||
// next request instead, which is ordered by the request pipeline and needs no
|
||||
// cooperation from the client. What this call still earns its place for is
|
||||
// the server-set cookie the route emits in reply: Safari's ITP caps the
|
||||
// script-written copy at 7 days, and a Set-Cookie is exempt.
|
||||
function persistLocaleToServer(newLocale: Locale) {
|
||||
void apiFetchRaw("/api/auth/me/locale", {
|
||||
method: "PUT",
|
||||
|
||||
Reference in new issue
Block a user