Skip to content

🐛 Fix active-users HTML parse errors on Netlify #4325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clubanderson merged 1 commit into from
Apr 2, 2026
Merged

🐛 Fix active-users HTML parse errors on Netlify #4325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions web/src/hooks/__tests__/useActiveUsers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ describe('useActiveUsers', () => {
localStorage.clear()
vi.clearAllMocks()
vi.spyOn(globalThis, 'fetch').mockResolvedValue(
new Response(JSON.stringify({ activeUsers: 5, totalConnections: 8 }), { status: 200 })
new Response(JSON.stringify({ activeUsers: 5, totalConnections: 8 }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
})
)
})

Expand Down Expand Up @@ -160,7 +163,10 @@ describe('useActiveUsers', () => {

// Now fix fetch and refetch
vi.mocked(fetch).mockResolvedValue(
new Response(JSON.stringify({ activeUsers: 3, totalConnections: 5 }), { status: 200 })
new Response(JSON.stringify({ activeUsers: 3, totalConnections: 5 }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
})
)

act(() => { result.current.refetch() })
Expand All @@ -175,7 +181,10 @@ describe('useActiveUsers', () => {
// --- Updates counts when API returns new data ---
it('updates active user counts when API returns new data', async () => {
vi.mocked(fetch).mockResolvedValue(
new Response(JSON.stringify({ activeUsers: 10, totalConnections: 15 }), { status: 200 })
new Response(JSON.stringify({ activeUsers: 10, totalConnections: 15 }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
})
)

const { result } = renderHook(() => useActiveUsers())
Expand All @@ -187,6 +196,22 @@ describe('useActiveUsers', () => {
})
})

// --- Handles HTML fallback response (Netlify SPA catch-all) ---
it('handles HTML response without SyntaxError (Netlify SPA fallback)', async () => {
vi.mocked(fetch).mockResolvedValue(
new Response('<!doctype html><html><body>SPA</body></html>', {
status: 200,
headers: { 'Content-Type': 'text/html' },
})
)

const { result } = renderHook(() => useActiveUsers())
await act(async () => { await vi.advanceTimersByTimeAsync(100) })

// Should not crash or throw SyntaxError — gracefully treated as error
expect(typeof result.current.activeUsers).toBe('number')
})
Comment on lines +199 to +213
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new HTML-fallback test doesn’t actually validate the behavior introduced by this PR. It would have passed even before the isJsonResponse() guard because resp.json().catch(() => null) already prevents the SyntaxError from failing the test. Consider asserting that json() is not called for a text/html response (e.g., return a minimal Response-like mock with headers.get() and a json spy) so the test fails if JSON parsing is attempted again.

Copilot generated this review using guidance from repository custom instructions.

// --- Handles invalid JSON gracefully ---
it('handles invalid JSON response without crashing', async () => {
vi.mocked(fetch).mockResolvedValue(
Expand Down
16 changes: 16 additions & 0 deletions web/src/hooks/useActiveUsers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ const RECOVERY_DELAY = 30_000 // Retry after circuit breaker trips
/** Timeout for fetch() call to the active-users endpoint */
const ACTIVE_USERS_FETCH_TIMEOUT_MS = 5_000

/**
* Guard against non-JSON responses (e.g. Netlify SPA catch-all returning index.html).
* On Netlify without a Go backend, API calls can fall through to the `/* -> /index.html`
* redirect if MSW hasn't registered yet or the Netlify Function fails. The response
* has status 200 but content-type text/html, causing `response.json()` to throw
* `SyntaxError: Unexpected token '<'`. Checking content-type prevents the parse attempt.
*/
function isJsonResponse(resp: Response): boolean {
const ct = resp.headers.get('content-type') || ''
return ct.includes('application/json')
}

// Singleton state to share across all hook instances
let sharedInfo: ActiveUsersInfo = {
activeUsers: 0,
Expand Down Expand Up @@ -177,6 +189,10 @@ async function fetchActiveUsers() {
try {
const resp = await fetch('/api/active-users', { signal: AbortSignal.timeout(ACTIVE_USERS_FETCH_TIMEOUT_MS) })
if (!resp.ok) throw new Error(`HTTP ${resp.status}`)
// Guard: if the response is HTML (e.g. Netlify SPA catch-all returning
// index.html because MSW hasn't intercepted yet), skip JSON parsing
// entirely to avoid SyntaxError: Unexpected token '<' console noise.
if (!isJsonResponse(resp)) throw new Error('Non-JSON response (likely HTML fallback)')
// Use .catch() on .json() to prevent Firefox from firing unhandledrejection
// before the outer try/catch processes the rejection (microtask timing issue).
const data = await resp.json().catch(() => null) as ActiveUsersInfo | null
Expand Down
Loading