Skip to content
73 changes: 73 additions & 0 deletions __tests__/lib/require-json-response.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* @jest-environment node
*/

/**
* FAMILIARISE_WEB-1C — a consultee's appointments page sent Sentry
* `SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON`, and
* put that same string in the toast. A Netlify crash page, a function timeout
* and a middleware redirect to sign-in all answer with HTML, and the redirect
* answers with it at status 200 — so `res.ok` alone never caught it.
*
* What this pins: the status survives to the caller. The cancel dialog reads
* `status === 409` to tell "the booking already changed state, refreshing" from
* a real failure; lose that and a lost CAS race starts looking like a crash.
*/

import { ApiResponseError, requireJsonResponse } from "@/lib/fetch-helpers";

const HTML = "<!DOCTYPE html><html><body>Gateway error</body></html>";

function htmlResponse(status: number): Response {
return new Response(HTML, {
status,
headers: { "content-type": "text/html; charset=utf-8" },
});
}

function jsonResponse(status: number, body: unknown): Response {
return new Response(JSON.stringify(body), {
status,
headers: { "content-type": "application/json" },
});
}

describe("requireJsonResponse", () => {
it("turns an HTML error page into a typed error carrying the status", async () => {
await expect(
requireJsonResponse(htmlResponse(502), "Failed to cancel appointment"),
).rejects.toMatchObject({
name: "ApiResponseError",
status: 502,
message: "Failed to cancel appointment (HTTP 502)",
});
});

it("rejects an HTML body served at 200 — a followed sign-in redirect", async () => {
const error = await requireJsonResponse(
htmlResponse(200),
"Events fetch failed",
).catch((e: unknown) => e);
expect(error).toBeInstanceOf(ApiResponseError);
expect((error as Error).name).not.toBe("SyntaxError");
expect((error as ApiResponseError).status).toBe(200);
});

it("keeps the 409 the cancel dialog branches on, and the server's own message", async () => {
await expect(
requireJsonResponse(
jsonResponse(409, { error: "This booking changed state" }),
"Failed to cancel appointment",
),
).rejects.toMatchObject({
status: 409,
message: "This booking changed state",
});
});

it("returns the parsed body on a 2xx JSON response", async () => {
await expect(
requireJsonResponse(jsonResponse(200, { refund: { status: "FAILED" } })),
).resolves.toEqual({ refund: { status: "FAILED" } });
});
});
19 changes: 16 additions & 3 deletions actions/stream/chat/stream.action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { streamLogger } from "@/lib/stream-logger";
import { getSession } from "@/lib/auth-server";
import { isPrivileged } from "@/lib/auth-helpers";
import { markExpected } from "@/lib/observability/expected";
import * as Sentry from "@sentry/nextjs";

// Token expiry for both chat and video (1 hour)
Expand All @@ -29,7 +30,13 @@ async function assertCanMintToken(forUserId: string): Promise<void> {
// expires (#899).
const session = await getSession(true);
if (!session?.user?.id) {
throw new Error("Unauthorized: sign in to request a Stream token");
// The connector now gates the mint on its own session, so reaching here is
// a tab whose cookie expired while it sat open — an answer, not a fault.
// The throw stays as the backstop; the marker keeps it off the error feed
// (FAMILIARISE_WEB-10).
throw markExpected(
new Error("Unauthorized: sign in to request a Stream token"),
);
}
// Never mint for a banned/suspended user (#693).
if (session.user.banned) {
Expand Down Expand Up @@ -66,7 +73,10 @@ export async function tokenProvider(userId: string): Promise<string> {
streamLogger.error("Failed to generate video token", error, {
userId: validatedUserId,
});
Sentry.captureException(error instanceof Error ? error : new Error(String(error)), { tags: { subsystem: "stream" } });
Sentry.captureException(
error instanceof Error ? error : new Error(String(error)),
{ tags: { subsystem: "stream" } },
);
throw error;
}
}
Expand Down Expand Up @@ -97,7 +107,10 @@ export async function chatTokenProvider(userId: string): Promise<string> {
streamLogger.error("Failed to generate chat token", error, {
userId: validatedUserId,
});
Sentry.captureException(error instanceof Error ? error : new Error(String(error)), { tags: { subsystem: "stream" } });
Sentry.captureException(
error instanceof Error ? error : new Error(String(error)),
{ tags: { subsystem: "stream" } },
);
throw error;
}
}
85 changes: 51 additions & 34 deletions components/appointments/consultee/useEventActions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"use client";

import { useState } from "react";
import * as Sentry from "@sentry/nextjs";
import { ApiResponseError, requireJsonResponse } from "@/lib/fetch-helpers";
import { reportSentryError } from "@/lib/observability/report";
import { useToast } from "@/hooks/use-toast";
import { useParams } from "next/navigation";
import { useQueryClient } from "@tanstack/react-query";
Expand All @@ -23,6 +24,26 @@ interface UseEventActionsOptions {
consulteeId?: string;
}

/**
* One report shape for both write actions on this page.
*
* These two catch blocks used to send whatever they caught at error level, and
* because both parsed the body before checking `res.ok`, an HTML error page
* arrived as `SyntaxError: Unexpected token '<'` — in Sentry and in the toast
* (FAMILIARISE_WEB-1C). With the status carried on the error, a 4xx is a server
* ANSWER rather than a fault, and everything with a status reports at warning.
*/
function reportActionFailure(error: unknown, op: string): void {
const status = error instanceof ApiResponseError ? error.status : undefined;
reportSentryError(error, {
subsystem: "client",
op,
expected: status !== undefined && status >= 400 && status < 500,
...(status !== undefined ? { level: "warning" as const } : {}),
extra: { httpStatus: status },
});
}

/**
* Which of the three reschedule outcomes happened.
*
Expand Down Expand Up @@ -197,15 +218,17 @@ export function useEventActions({
const response = await fetch(url, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: Object.keys(payload).length
? JSON.stringify(payload)
: undefined,
body: Object.keys(payload).length ? JSON.stringify(payload) : undefined,
});

const data = await response.json();
if (!response.ok) {
throw new Error(data.error || "Failed to request reschedule");
}
const data = (await requireJsonResponse(
response,
"Failed to request reschedule",
)) as {
sessionsAffected?: number;
slotsAffected?: number;
autoConfirmed?: boolean;
};

// #448 — show the SESSION count, not the slot count: a 1-hour session is
// 2 × 30-min slots, so the old slotsAffected label read "2 sessions" for a
Expand All @@ -224,10 +247,7 @@ export function useEventActions({
invalidateBookingData();
return true;
} catch (error) {
Sentry.captureException(
error instanceof Error ? error : new Error(String(error)),
{ tags: { subsystem: "client" } },
);
reportActionFailure(error, "appointment.reschedule");
console.error("Error requesting reschedule:", error);
toast({
title: "Error",
Expand Down Expand Up @@ -262,42 +282,39 @@ export function useEventActions({
`/api/appointments/${appointmentId}/cancel`,
{ method: "POST", headers: { "Content-Type": "application/json" } },
);
const data = await response.json();
if (!response.ok) {
// 409 = the CAS transition guard matched zero rows: the booking
// already changed state (double-cancel, consultant approved a
// stale tab, …). The list refresh IS the answer — not an error.
if (response.status === 409) {
toast({
title: "Booking already updated",
description:
"This booking changed state in the meantime — refreshing.",
});
setShowCancelDialog(false);
invalidateBookingData();
return;
}
throw new Error(data.error || "Failed to cancel appointment");
}
const data = (await requireJsonResponse(
response,
"Failed to cancel appointment",
)) as { refund?: CancelRefund };
// The route reports what happened to the money on `refund`. Discarding
// it meant a cancellation that refunded nothing — or failed to refund —
// looked exactly like one that paid out in full.
toast({
title: "Appointment cancelled",
description: [
`Your ${type.toLowerCase()} "${title}" has been cancelled.`,
describeRefund(data.refund),
describeRefund(data.refund ?? null),
]
.filter(Boolean)
.join(" "),
});
setShowCancelDialog(false);
invalidateBookingData();
} catch (error) {
Sentry.captureException(
error instanceof Error ? error : new Error(String(error)),
{ tags: { subsystem: "client" } },
);
// 409 = the CAS transition guard matched zero rows: the booking already
// changed state (double-cancel, consultant approved a stale tab, …).
// The list refresh IS the answer — not an error.
if (error instanceof ApiResponseError && error.status === 409) {
toast({
title: "Booking already updated",
description:
"This booking changed state in the meantime — refreshing.",
});
setShowCancelDialog(false);
invalidateBookingData();
return;
}
reportActionFailure(error, "appointment.cancel");
console.error("Error cancelling appointment:", error);
toast({
title: "Error",
Expand Down
8 changes: 8 additions & 0 deletions docs/booking/05-troubleshooting-and-changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ Wave 6 picks up the follow-ups that wave 5 left marked in the code. Each PR appe
- **Creation is now a history row.** A freshly created request had never transitioned, so it had no rows at all and the staff timeline answered "nothing has moved on this booking yet" for every new booking on production. A new `appendCreationHistory` helper writes one opening row in the same transaction as the create, from the literal `"CREATED"` to whatever status the row was born in. The literal matters: `appendHistory` renders a missing from-status as `"UNKNOWN"`, which on this surface means a concurrent writer moved the row between the pre-read and the update, and creation is emphatically not that.
- **Three creation paths write it.** The direct-checkout consultation and subscription handlers already run inside the checkout transaction and simply append after the appointment exists. The request-for-approval route did not have a transaction at all — its nested create was atomic on its own — so the create and the audit row are now wrapped in one, budgeted well inside the sixty-second slot lock the route already holds. The capture webhook's own creators in `lib/payments/webhooks/handlers.ts` are deliberately out of scope for this PR. They are the legacy fallback that builds a booking when the payment carries no appointment because checkout never made one, so a booking born that way still opens with no creation row and its timeline starts at its first real transition.

### PR F — Sentry findings from the 2026-09-03 releases (`fix/sentry-2026-09-03-findings`)

- **FAMILIARISE_WEB-1B — a rejected Novu trigger reached us as a schema complaint about the SDK's own error envelope.** The booking-request notification on `POST /api/slots/request-for-approval` failed, but what arrived in Sentry was a `ZodError` saying the field `errors` was required. The `@novu/api` schema for a 422 response requires an `errors` record, and both of the 422s Novu documents for the trigger endpoint — an unknown or unpublished workflow, and an idempotency key reused with a different body — answer with `statusCode` and `message` only. The SDK therefore threw while parsing the rejection and the real status never surfaced. No published version of the SDK relaxes that field, checked through 3.19.1, so the wrapper in `lib/novu/service.ts` now reads the status off the thrown error instead. A non-2xx is reported with its status, Novu's own message and the workflow id; a 2xx the SDK merely failed to parse is treated as an accepted send rather than a lost notification, because the trigger is already queued in that case.
- **FAMILIARISE_WEB-1C — the consultee appointments page parsed an HTML error page as JSON.** Both write actions on that page read the body before checking the response, so a Netlify crash page or a followed redirect to sign-in became `SyntaxError: Unexpected token '<'` — in the toast the consultee read, and in the error feed. A new `requireJsonResponse` in `lib/fetch-helpers.ts` checks the status and the content type before parsing and throws the existing typed `ApiResponseError`, which carries the status. The dashboard query factory and the cancel and reschedule actions now go through it, so the page shows its normal error state, the cancel dialog still recognises the 409 that means the booking changed state underneath it, and Sentry receives a warning naming the status.
- **FAMILIARISE_WEB-1D — a dropped connection paged as an error.** `useNovuSubscriberSync` reported every failure at error level, including `TypeError: Failed to fetch`, which carries no status at all and simply means the request never got an answer. React Query already retries the sync and nothing user-visible depends on it, so a transport fault is no longer reported and only a real HTTP answer is: a 4xx as a modelled outcome, a 5xx as a fault, both at warning with the status attached. The hook also skips the call outright while the browser reports itself offline, which React Query's own `networkMode` does not cover — its online manager starts optimistically and only ever flips on the window connectivity events, so a tab opened while already offline fires the request anyway.
- **FAMILIARISE_WEB-10 — a signed-out tab kept asking for a Stream token.** The token action refuses to mint without a session, and a dashboard left open past its cookie's expiry called it regardless. The connector now consults its own session and skips the mint once the session is known to be absent, treating a still-loading session as permitted so the first token request keeps its place off the join path. The server-side refusal stays as the backstop, marked expected so it lands at warning rather than on the error feed; `lib/observability/expected.ts` carries that marker and `sentry.shared.config.ts` stamps it, which is the only way to reach an error captured by Next's request-error hook.
- **FAMILIARISE_WEB-9 is deliberately untouched.** The Prisma connection timeouts are the instance-boot stall tracked under #1124, not a defect in any of the above, and the pool settings are left alone.

## Changelog: 2026-09-02 — wave 5

The wave-5 train (#1319) reconciles the original booking and maintenance audit briefs against everything that shipped in waves 1–4 and closes the residuals that survived. Each PR appends its own bullets here.
Expand Down
52 changes: 41 additions & 11 deletions hooks/useNovuSubscriberSync.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,39 @@
"use client";

import { useSyncExternalStore } from "react";
import { useQuery } from "@tanstack/react-query";
import { reportSentryError } from "@/lib/observability/report";
import { useSession } from "@/lib/auth-client";

/** Re-render on the browser's own connectivity events, so `enabled` can't latch. */
function subscribeToConnectivity(notify: () => void): () => void {
window.addEventListener("online", notify);
window.addEventListener("offline", notify);
return () => {
window.removeEventListener("online", notify);
window.removeEventListener("offline", notify);
};
}

/**
* Auto-syncs the current user as a Novu subscriber.
* Called once per dashboard session. Uses a long staleTime to avoid repeat calls.
*/
export function useNovuSubscriberSync() {
const { data: session } = useSession();

// React Query's `networkMode: "online"` already pauses a fetch while offline,
// but its `onlineManager` starts at `online = true` and only ever flips on the
// window online/offline EVENTS — it never reads `navigator.onLine`. So a tab
// opened while already offline sails past that guard and fires the request,
// which is one of the ways this hook produced `TypeError: Failed to fetch`
// (FAMILIARISE_WEB-1D). Reading the flag directly closes that opening.
const isOnline = useSyncExternalStore(
subscribeToConnectivity,
() => navigator.onLine !== false,
() => true,
);

return useQuery({
queryKey: ["novu-subscriber-sync", session?.user?.id],
queryFn: async () => {
Expand All @@ -29,20 +52,27 @@ export function useNovuSubscriberSync() {
error && typeof error === "object" && "httpStatus" in error
? (error as { httpStatus?: number }).httpStatus
: undefined;
const expected =
typeof httpStatus === "number" && httpStatus >= 400 && httpStatus < 500;
// React Query retries this (retry: 1); a failed sync just delays
// the subscriber row, nothing user-visible breaks. A 5xx/network
// fault is still worth alerting on even though retry papers over it.
reportSentryError(error, {
subsystem: "novu",
expected,
extra: { httpStatus },
});
// No status at all means the request never got an answer: a dropped
// connection, a tab navigating away mid-flight, a captive portal. React
// Query retries it and nothing user-visible breaks, so it is noise
// rather than a fault and used to page as `TypeError: Failed to fetch`
// at error level (FAMILIARISE_WEB-1D). A real answer still reports:
// 4xx is a modelled outcome, 5xx is worth alerting on even though the
// retry papers over it.
if (typeof httpStatus === "number") {
reportSentryError(error, {
subsystem: "novu",
op: "subscriber.sync",
expected: httpStatus >= 400 && httpStatus < 500,
level: "warning",
extra: { httpStatus },
});
}
throw error;
}
},
enabled: !!session?.user?.id && !!process.env.NEXT_PUBLIC_NOVU_APP_ID,
enabled:
isOnline && !!session?.user?.id && !!process.env.NEXT_PUBLIC_NOVU_APP_ID,
staleTime: 30 * 60 * 1000, // 30 minutes
gcTime: 60 * 60 * 1000, // 1 hour
retry: 1,
Expand Down
15 changes: 10 additions & 5 deletions lib/dashboard-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
PlannerClassEvent,
} from "@/types/planner-events";
import type { RecordingData } from "@/types/recording";
import { requireJsonResponse } from "@/lib/fetch-helpers";

// =============================================================================
// Types
Expand Down Expand Up @@ -56,11 +57,15 @@ async function fetchWithErrorHandling<T>(
errorPrefix: string,
): Promise<T> {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`${errorPrefix}: ${response.statusText}`);
}
const json = await response.json();
return json.data ?? json;
// `requireJsonResponse` carries the status on the thrown error and refuses to
// parse an HTML body, so a Netlify crash page or a sign-in redirect reaches
// the dashboard as "… (HTTP 502)" rather than a JSON syntax error
// (FAMILIARISE_WEB-1C). `statusText` is empty over HTTP/2, which is why the
// old message ended in a colon and nothing.
const json = (await requireJsonResponse(response, errorPrefix)) as {
data?: T;
} | null;
return (json?.data ?? json) as T;
}

// Consultant fetchers
Expand Down
Loading
Loading