From 9905fe4a05300b2850f5f7ab179ab16ce7633202 Mon Sep 17 00:00:00 2001 From: pataniaeli Date: Fri, 28 Aug 2026 17:10:09 -0400 Subject: [PATCH 1/2] fix: stop an error response from crashing the page (#53) A user hit a broken SGA Spaces page. /api/spaces returned a transient 401 and the page died with "l.find is not a function"; a reload cleared it. fetch('/api/spaces') .then(r => r.json()) .then((data: Space[]) => setSpaces(data)) The annotation is a promise the code cannot keep. An error response is still valid JSON, so r.json() resolved happily with { error: 'Unauthorized' }, that object went into state typed as Space[], and the page died on the next spaces.find(). `data.length > 0` had already passed silently -- undefined > 0 is just false -- so nothing failed until the render. TypeScript cannot catch this: the cast is asserted at a boundary where the shape is only known at runtime. Eight call sites shared the pattern. Four degraded harmlessly on their own (data.groups || [], (requests ?? [])), but two more could crash the same way and one was worse than a crash: sga-spaces/page.tsx the reported failure sga-spaces-tab.tsx .then(setSpaces) -- identical shape, admin view booking-settings-tab every field falls back to a hardcoded default, so an error body silently populated the form with 0/0/24 and an admin pressing Save would have written those over the real booking limits settings-modal error body cast to Settings, offered back for saving archive-tab no .catch at all, so a rejected fetch left setLoading(false) unreached and the tab hung on its skeleton getJson/getJsonArray take the fallback as an argument, which makes the failure case impossible to leave out and keeps the return type honest. The Spaces page handles its response directly instead, because there an empty list is a meaningful answer -- "no rooms configured" -- and quietly substituting one for a failed request would state something untrue. It now shows a short error with a Try again button, which is the same remedy the reporter found. Not fixed here: why /api/spaces 401s in the first place. Both endpoints in the report use the plain getAuthedUser path, no user is missing a profile row and no sessions are revoked, so this is a transient auth failure rather than a regression. What is fixed is that it can no longer take the page down. Co-Authored-By: Claude Opus 5 --- app/(dashboard)/administrator/archive-tab.tsx | 6 +- .../administrator/booking-settings-tab.tsx | 10 ++- .../administrator/one-time-form.tsx | 5 +- .../administrator/sga-spaces-tab.tsx | 3 +- .../administrator/tabling-form.tsx | 5 +- app/(dashboard)/administrator/weekly-form.tsx | 5 +- app/(dashboard)/settings-modal.tsx | 14 ++-- app/(dashboard)/sga-spaces/page.tsx | 64 ++++++++++++++++--- lib/fetch-json.ts | 48 ++++++++++++++ 9 files changed, 133 insertions(+), 27 deletions(-) create mode 100644 lib/fetch-json.ts diff --git a/app/(dashboard)/administrator/archive-tab.tsx b/app/(dashboard)/administrator/archive-tab.tsx index 3d397c6..aeebe28 100644 --- a/app/(dashboard)/administrator/archive-tab.tsx +++ b/app/(dashboard)/administrator/archive-tab.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { Skeleton } from '@/app/_components/skeleton' function ArchiveTabSkeleton() { @@ -266,8 +267,9 @@ export default function ArchiveTab() { const [loading, setLoading] = useState(true) useEffect(() => { - fetch('/api/administrator/archive') - .then(r => r.json()) + // Also fixes a hang: the old chain had no .catch, so a rejected fetch left + // setLoading(false) unreached and the tab stuck on its skeleton forever. + getJson<{ groups?: SemesterGroup[] }>('/api/administrator/archive', {}) .then(data => { setGroups(data.groups || []) setLoading(false) diff --git a/app/(dashboard)/administrator/booking-settings-tab.tsx b/app/(dashboard)/administrator/booking-settings-tab.tsx index 23c75e9..0660242 100644 --- a/app/(dashboard)/administrator/booking-settings-tab.tsx +++ b/app/(dashboard)/administrator/booking-settings-tab.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { createBrowserClient } from '@supabase/ssr' import { getAuthedUser } from '@/lib/auth' import { Skeleton } from '@/app/_components/skeleton' @@ -107,9 +108,14 @@ export default function BookingSettingsTab() { const [deletePermissionChecked, setDeletePermissionChecked] = useState(false) useEffect(() => { - fetch('/api/administrator/settings') - .then(r => r.json()) + // The stakes here are higher than a blank screen. Every field below falls + // back to a hardcoded default when the value is missing, so an error body + // would silently populate the form with 0 / 0 / 24 -- and an admin who then + // pressed Save would write those over the real booking limits. Failing the + // load outright keeps the tab on its skeleton instead. + getJson | null>('/api/administrator/settings', null) .then(data => { + if (!data) return setMinDaysRoom(data.min_days_advance_room ?? 0) setMinDaysTabling(data.min_days_advance_tabling ?? 0) setMinHoursSpaces(data.min_hours_advance_spaces ?? 24) diff --git a/app/(dashboard)/administrator/one-time-form.tsx b/app/(dashboard)/administrator/one-time-form.tsx index ba93878..416bc64 100644 --- a/app/(dashboard)/administrator/one-time-form.tsx +++ b/app/(dashboard)/administrator/one-time-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -100,14 +101,12 @@ export default function OneTimeForm({ bodies, semesters, onClose, onSuccess }: O const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'One-Time Room') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/administrator/sga-spaces-tab.tsx b/app/(dashboard)/administrator/sga-spaces-tab.tsx index 69e17bf..ae9ac9c 100644 --- a/app/(dashboard)/administrator/sga-spaces-tab.tsx +++ b/app/(dashboard)/administrator/sga-spaces-tab.tsx @@ -4,6 +4,7 @@ import { useState, useEffect, useCallback } from 'react' import TimePicker from './time-picker' import { Skeleton } from '@/app/_components/skeleton' import DateField from '@/app/_components/date-field' +import { getJsonArray } from '@/lib/fetch-json' function TableSkeleton({ cols }: { cols: number }) { return ( @@ -155,7 +156,7 @@ export default function SGASpacesTab() { const [spaces, setSpaces] = useState([]) useEffect(() => { - fetch('/api/spaces').then(r => r.json()).then(setSpaces) + getJsonArray('/api/spaces').then(setSpaces) }, []) return ( diff --git a/app/(dashboard)/administrator/tabling-form.tsx b/app/(dashboard)/administrator/tabling-form.tsx index ce97e92..9745489 100644 --- a/app/(dashboard)/administrator/tabling-form.tsx +++ b/app/(dashboard)/administrator/tabling-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -104,14 +105,12 @@ export default function TablingForm({ bodies, semesters, onClose, onSuccess }: T const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'Tabling') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/administrator/weekly-form.tsx b/app/(dashboard)/administrator/weekly-form.tsx index ba61929..ae8b27b 100644 --- a/app/(dashboard)/administrator/weekly-form.tsx +++ b/app/(dashboard)/administrator/weekly-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -93,14 +94,12 @@ export default function WeeklyForm({ bodies, semesters, onClose, onSuccess }: We const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'Weekly Room') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/settings-modal.tsx b/app/(dashboard)/settings-modal.tsx index 2e35684..c3d53b5 100644 --- a/app/(dashboard)/settings-modal.tsx +++ b/app/(dashboard)/settings-modal.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { Skeleton } from '@/app/_components/skeleton' import { getPrefsForRole, EMAIL_PREF_LABELS, EmailPrefKey } from '@/lib/email-preferences' @@ -57,16 +58,19 @@ export default function SettingsModal({ onClose, cachedSettings, onSettingsLoade const loadSettings = () => { setLoading(true) - fetch('/api/me/settings') - .then(r => r.json()) - .then((data: Settings) => { + // null rather than a blank Settings object on failure. Every consumer below + // is already guarded on `settings &&`, so null renders as empty sections, + // whereas an error body cast to Settings would put undefined into the name + // field and the preference toggles and invite the user to save it back. + getJson('/api/me/settings', null) + .then(data => { + setLoading(false) + if (!data) return setSettings(data) setNameValue(data.full_name ?? '') setSelectedBodyId('') - setLoading(false) onSettingsLoaded?.(data) }) - .catch(() => setLoading(false)) } useEffect(() => { if (cachedSettings == null) loadSettings() }, []) diff --git a/app/(dashboard)/sga-spaces/page.tsx b/app/(dashboard)/sga-spaces/page.tsx index 89fe030..2cfa294 100644 --- a/app/(dashboard)/sga-spaces/page.tsx +++ b/app/(dashboard)/sga-spaces/page.tsx @@ -139,6 +139,7 @@ function CalendarSkeleton() { export default function SGASpacesPage() { const [spaces, setSpaces] = useState([]) const [spacesLoading, setSpacesLoading] = useState(true) + const [spacesError, setSpacesError] = useState(false) const [selectedSpaceId, setSelectedSpaceId] = useState(null) const [bookings, setBookings] = useState([]) const [blackouts, setBlackouts] = useState([]) @@ -184,16 +185,40 @@ export default function SGASpacesPage() { }) }, []) - useEffect(() => { - fetch('/api/spaces') - .then(r => r.json()) - .then((data: Space[]) => { - setSpaces(data) - if (data.length > 0) setSelectedSpaceId(data[0].id) - }) - .finally(() => setSpacesLoading(false)) + // Handled explicitly rather than via getJson: on this page an empty list is a + // meaningful answer ("no rooms configured"), so quietly substituting one for a + // failed request would tell the user something untrue. + // + // Issue #53: /api/spaces returned a transient 401, r.json() resolved happily + // with { error: 'Unauthorized' }, that object went into `spaces`, and the page + // died on the next spaces.find(). The response shape is now checked before it + // is trusted, so a bad response can only ever produce an error state. + const loadSpaces = useCallback(async () => { + setSpacesLoading(true) + setSpacesError(false) + try { + const res = await fetch('/api/spaces') + if (!res.ok) throw new Error(`/api/spaces responded ${res.status}`) + + const data: unknown = await res.json() + if (!Array.isArray(data)) throw new Error('/api/spaces did not return a list') + + const list = data as Space[] + setSpaces(list) + if (list.length > 0) setSelectedSpaceId(prev => prev || list[0].id) + } catch (err) { + console.error('Failed to load spaces:', err) + setSpaces([]) + setSpacesError(true) + } finally { + setSpacesLoading(false) + } }, []) + useEffect(() => { + loadSpaces() + }, [loadSpaces]) + // Remaining hours only change when the user books or cancels -- not when the // calendar week changes. Keying this off `bookings` refetched it on every week // navigation and fired it twice on mount (once with the initial empty array). @@ -279,6 +304,29 @@ export default function SGASpacesPage() { return } + // Distinct from "no rooms configured", which renders the normal empty calendar + // below. The failure that prompted this was transient and a reload cleared it, + // so the useful thing to offer is another attempt. + if (spacesError) { + return ( +
+

SGA Spaces

+
+

Couldn't load the spaces

+

+ Something went wrong reaching the server. This is usually temporary. +

+ +
+
+ ) + } + return ( <>
diff --git a/lib/fetch-json.ts b/lib/fetch-json.ts new file mode 100644 index 0000000..50078e4 --- /dev/null +++ b/lib/fetch-json.ts @@ -0,0 +1,48 @@ +/** + * GET a JSON endpoint, falling back to a known-good value when the request does + * not succeed. + * + * Written for a bug that took down the SGA Spaces page (issue #53). The call + * sites all looked like this: + * + * fetch('/api/spaces') + * .then(r => r.json()) + * .then((data: Space[]) => setSpaces(data)) + * + * The annotation is a promise the code cannot keep. An error response is still + * valid JSON, so `r.json()` resolves happily with `{ error: 'Unauthorized' }`, + * that object lands in state typed as an array, and the page dies on the next + * `spaces.find(...)`. TypeScript cannot catch it: the cast is asserted at a + * boundary where the real shape is only known at runtime. + * + * Passing the fallback in makes the failure case impossible to leave out, and + * keeps the type honest -- the return is whatever the caller was already + * prepared to render. + * + * Silent on failure by design: these are background reads whose call sites have + * no error UI. Where "the request failed" is different from "there is nothing + * here" -- as it is on the Spaces page, where an empty list reads as "no rooms + * exist" -- handle the response explicitly instead of reaching for this. + */ +export async function getJson(url: string, fallback: T): Promise { + try { + const res = await fetch(url) + if (!res.ok) return fallback + const data = await res.json() + return (data ?? fallback) as T + } catch { + // Network failure, or a body that is not JSON at all (an HTML error page + // from a proxy, say). Both are "no data", same as a non-2xx. + return fallback + } +} + +/** + * As getJson, but guarantees an array. Use for list endpoints whose result is + * indexed, mapped or searched -- an object arriving where an array is expected + * is precisely what crashed issue #53. + */ +export async function getJsonArray(url: string): Promise { + const data = await getJson(url, []) + return Array.isArray(data) ? (data as T[]) : [] +} From ec9ccc6034a282b64a90c46d552adf3e47dfe90b Mon Sep 17 00:00:00 2001 From: pataniaeli Date: Fri, 28 Aug 2026 17:39:30 -0400 Subject: [PATCH 2/2] fix: sign out this device only, not every session the user holds signOut() defaults to scope 'global' -- confirmed in auth-js: async _signOut({ scope } = { scope: 'global' }) which revokes every refresh token the user holds anywhere. So the 44-minute idle timer firing on someone's laptop also ended the session on their phone, in their other tab, and the copy the Edge middleware refreshes. Each of those then failed its next refresh with a 400 and began serving 401s, which is how a page that was working a moment ago starts returning Unauthorized. Supabase's logs over 24h: of 28 refresh_token grants, 9 returned 400 -- 5 from browsers and 4 from the Edge middleware. Alongside 28 logouts across ~27 users, and only 3 of 24 refresh tokens ever having a parent (so rotation is barely occurring, meaning a 400 is a revoked token rather than a rotation race). The four sign-outs in dashboard-shell -- the Sign Out button and the three idle paths -- now use scope 'local'. That is what signing out means on a shared dashboard: this browser, not every device I own. The four that mean "this account may not be used" keep the global scope, and say so in a comment: deactivation in force-sign-out.tsx, LoginCard and onboarding, plus an expired invite. Ending every session is the point there. This is a user-facing fix regardless of the 401s -- signing out at a library machine should not log you out on your phone. Not claimed: that this accounts for every 401. Vercel's runtime logs are not readable with the token I have, so the reported failure could not be tied to a specific revocation. Some 400s also fall in hours with no logouts at all. Co-Authored-By: Claude Opus 5 --- app/(dashboard)/dashboard-shell.tsx | 25 +++++++++++++++++++++---- app/(dashboard)/force-sign-out.tsx | 3 +++ app/_components/LoginCard.tsx | 3 +++ app/onboarding/page.tsx | 3 +++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/(dashboard)/dashboard-shell.tsx b/app/(dashboard)/dashboard-shell.tsx index 661de3f..55c9442 100644 --- a/app/(dashboard)/dashboard-shell.tsx +++ b/app/(dashboard)/dashboard-shell.tsx @@ -210,9 +210,26 @@ export default function DashboardShell({ load() }, [fetchDashboard]) + /** + * Ends the session on this device only. + * + * signOut() defaults to scope 'global', which revokes every refresh token the + * user holds anywhere. Signing out on a laptop therefore also killed the + * session on their phone, in their other tab, and the copy the Edge middleware + * refreshes -- and each of those then failed its next refresh with a 400 and + * started serving 401s. Over 24h, 9 of 28 refresh attempts were failing that + * way, split across the browser and the middleware. + * + * 'local' is what "Sign Out" means on a shared dashboard: this browser, not + * every device I own. The paths that mean "this account may not be used" -- + * deactivation in force-sign-out.tsx and LoginCard, an expired invite -- + * deliberately keep the global scope. + */ + const signOutThisDevice = () => supabase.auth.signOut({ scope: 'local' }) + const handleLogout = async () => { localStorage.removeItem('chambers_last_active') - await supabase.auth.signOut() + await signOutThisDevice() router.push('/') } @@ -227,7 +244,7 @@ export default function DashboardShell({ clearInterval(countdownIntervalRef.current!) countdownIntervalRef.current = null localStorage.removeItem('chambers_last_active') - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) } }, 1000) } @@ -251,7 +268,7 @@ export default function DashboardShell({ if (storedLastActive) { const elapsed = Date.now() - parseInt(storedLastActive, 10) if (elapsed >= IDLE_MS) { - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) return } } @@ -276,7 +293,7 @@ export default function DashboardShell({ if (elapsed >= IDLE_MS) { if (idleTimerRef.current) clearTimeout(idleTimerRef.current) if (countdownIntervalRef.current) clearInterval(countdownIntervalRef.current) - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) } } } diff --git a/app/(dashboard)/force-sign-out.tsx b/app/(dashboard)/force-sign-out.tsx index bd2d6c1..c133bab 100644 --- a/app/(dashboard)/force-sign-out.tsx +++ b/app/(dashboard)/force-sign-out.tsx @@ -19,6 +19,9 @@ export default function ForceSignOut() { useEffect(() => { const signOut = async () => { localStorage.removeItem('chambers_last_active') + // Global scope on purpose, unlike the sign-outs in dashboard-shell. This + // one runs because the account was deactivated, so every session it holds + // anywhere should end -- not just the one in this browser. await createClient().auth.signOut() router.replace('/') } diff --git a/app/_components/LoginCard.tsx b/app/_components/LoginCard.tsx index 9acbdac..3b030b6 100644 --- a/app/_components/LoginCard.tsx +++ b/app/_components/LoginCard.tsx @@ -53,6 +53,9 @@ export default function LoginCard() { .single() if (!profile?.is_active) { + // Global scope kept deliberately here and below: these mean the account + // may not be used at all, so every session it holds should end. An + // ordinary sign-out (dashboard-shell) is scoped 'local' instead. await supabase.auth.signOut() setError('Your account has been deactivated. Please contact an administrator.') setLoading(false) diff --git a/app/onboarding/page.tsx b/app/onboarding/page.tsx index 0cd5bf7..db7fc0b 100644 --- a/app/onboarding/page.tsx +++ b/app/onboarding/page.tsx @@ -45,6 +45,9 @@ export default function OnboardingPage() { .single() if (!profile?.is_active) { + // Global scope deliberately: the account is deactivated, so every + // session it holds should end, not only this browser's. See + // dashboard-shell for the ordinary 'local' sign-out. await supabase.auth.signOut() router.push('/') return