Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 31 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,37 @@ const config = [
'react-hooks/immutability': 'off',
},
},
// P1.5 — Discourage bare `fetch('/api/...')`.
// The team must migrate to `apiFetch<T>()` from `@/lib/api-client` so that
// 401 / 403 / 5xx / network failures are handled uniformly.
// Phase 1 (this PR): warn level, allow incremental migration.
// Phase 3 (final cleanup): upgrade to error and forbid merges that introduce
// new bare fetch('/api/...') sites.
// Selector rationale:
// - covers single-quoted, double-quoted, and template-literal forms
// - filters by /api prefix so cross-origin / external fetches stay untouched
// - exempts api-client.ts itself (the one allowed implementer)
{
files: ['src/**/*.{ts,tsx,js,jsx}'],
ignores: ['src/lib/api-client.ts'],
rules: {
'no-restricted-syntax': [
'warn',
{
selector:
"CallExpression[callee.name='fetch'] > Literal[value=/^\\/api\\//]",
message:
"Use apiFetch<T>() from '@/lib/api-client' instead of bare fetch('/api/...'). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly. See PR-api-client.md.",
},
{
selector:
"CallExpression[callee.name='fetch'] > TemplateLiteral.arguments:first-child[quasis.0.value.raw=/^\\/api\\//]",
message:
"Use apiFetch<T>() from '@/lib/api-client' instead of bare fetch(`/api/...`). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly.",
},
],
},
},
]

export default config
2 changes: 2 additions & 0 deletions src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { NextIntlClientProvider } from 'next-intl'
import { getLocale, getMessages } from 'next-intl/server'
import { THEME_IDS } from '@/lib/themes'
import { ThemeBackground } from '@/components/ui/theme-background'
import { AuthExpiredListener } from '@/components/auth-expired-listener'
import './globals.css'

const inter = Inter({
Expand Down Expand Up @@ -114,6 +115,7 @@ export default async function RootLayout({
disableTransitionOnChange
>
<ThemeBackground />
<AuthExpiredListener />
<div className="h-screen overflow-hidden bg-background text-foreground">
{children}
</div>
Expand Down
33 changes: 33 additions & 0 deletions src/components/auth-expired-listener.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use client'

import { useEffect } from 'react'

/**
* Listens for `mc:auth-expired` CustomEvent dispatched by `apiFetch()` when the
* server returns 401. The redirect to `/login?from=...` is already handled inside
* `apiFetch`; this listener exists so we have a single observability hook the
* team can extend (toast, telemetry, Sentry).
*
* Mounted once at the root layout. SSR-safe (effect runs only on the client).
*
* Why a separate component?
* - layout.tsx is a server component (uses `await headers()`); we cannot
* attach window listeners there directly.
* - Co-locating the listener with the api-client keeps the auth-failure
* contract in one place.
*/
export function AuthExpiredListener(): null {
useEffect(() => {
const onExpired = (e: Event) => {
const detail = (e as CustomEvent<{ path: string; status: number }>).detail
// No toast lib installed yet — log for now. Replace with sonner in next PR.
console.warn(
`[mc] session expired on ${detail?.path ?? 'unknown'} (status=${detail?.status ?? 401}), redirecting to /login`
)
}
window.addEventListener('mc:auth-expired', onExpired)
return () => window.removeEventListener('mc:auth-expired', onExpired)
}, [])

return null
}
40 changes: 19 additions & 21 deletions src/components/panels/multi-gateway-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useTranslations } from 'next-intl'
import { Button } from '@/components/ui/button'
import { useMissionControl } from '@/store'
import { useWebSocket } from '@/lib/websocket'
import { apiFetch } from '@/lib/api-client'

interface Gateway {
id: number
Expand Down Expand Up @@ -84,33 +85,29 @@ export function MultiGatewayPanel() {

const fetchGateways = useCallback(async () => {
try {
const res = await fetch('/api/gateways')
const data = await res.json()
const data = await apiFetch<{ gateways?: Gateway[] }>('/api/gateways')
setGateways(data.gateways || [])
} catch { /* ignore */ }
setLoading(false)
}, [])

const fetchDirectConnections = useCallback(async () => {
try {
const res = await fetch('/api/connect')
const data = await res.json()
const data = await apiFetch<{ connections?: DirectConnection[] }>('/api/connect')
setDirectConnections(data.connections || [])
} catch { /* ignore */ }
}, [])

const fetchDiscovered = useCallback(async () => {
try {
const res = await fetch('/api/gateways/discover')
const data = await res.json()
const data = await apiFetch<{ gateways?: DiscoveredGateway[] }>('/api/gateways/discover')
setDiscoveredGateways(data.gateways || [])
} catch { /* ignore */ }
}, [])

const fetchHistory = useCallback(async () => {
try {
const res = await fetch('/api/gateways/health/history')
const data = await res.json()
const data = await apiFetch<{ history?: GatewayHistory[] }>('/api/gateways/health/history')
const map: Record<number, GatewayHistory> = {}
for (const entry of data.history || []) {
map[entry.gatewayId] = entry
Expand Down Expand Up @@ -141,40 +138,41 @@ export function MultiGatewayPanel() {
!gateways.some(gatewayMatchesConnection)

const setPrimary = async (gw: Gateway) => {
await fetch('/api/gateways', {
await apiFetch('/api/gateways', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ id: gw.id, is_primary: 1 }),
})
}).catch(() => {})
fetchGateways()
fetchHistory()
}

const deleteGateway = async (id: number) => {
await fetch('/api/gateways', {
await apiFetch('/api/gateways', {
method: 'DELETE',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ id }),
})
}).catch(() => {})
fetchGateways()
fetchHistory()
}

const updateToken = async (gw: Gateway, token: string) => {
await fetch('/api/gateways', {
await apiFetch('/api/gateways', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ id: gw.id, token }),
})
}).catch(() => {})
fetchGateways()
}

const connectTo = async (gw: Gateway) => {
try {
const res = await fetch('/api/gateways/connect', {
const res = await apiFetch<Response>('/api/gateways/connect', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ id: gw.id }),
raw: true,
})
if (!res.ok) return
const payload = await res.json()
Expand All @@ -192,8 +190,7 @@ export function MultiGatewayPanel() {

const probeAll = async () => {
try {
const res = await fetch("/api/gateways/health", { method: "POST" })
const data = await res.json().catch(() => ({}))
const data = await apiFetch<{ results?: GatewayHealthProbe[] }>("/api/gateways/health", { method: "POST" })
const rows = Array.isArray(data?.results) ? data.results as GatewayHealthProbe[] : []
const mapped = new Map<number, GatewayHealthProbe>()
for (const row of rows) {
Expand All @@ -213,7 +210,7 @@ export function MultiGatewayPanel() {

const disconnectCli = async (connectionId: string) => {
try {
await fetch('/api/connect', {
await apiFetch('/api/connect', {
method: 'DELETE',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ connection_id: connectionId }),
Expand Down Expand Up @@ -350,7 +347,7 @@ export function MultiGatewayPanel() {
</div>
<Button
onClick={async () => {
await fetch('/api/gateways', {
await apiFetch('/api/gateways', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
Expand All @@ -359,7 +356,7 @@ export function MultiGatewayPanel() {
port: dg.port,
is_primary: false,
}),
})
}).catch(() => {})
fetchGateways()
fetchDiscovered()
}}
Expand Down Expand Up @@ -643,7 +640,7 @@ function AddGatewayForm({ onAdded, onCancel }: { onAdded: () => void; onCancel:
setSaving(true)

try {
const res = await fetch('/api/gateways', {
const res = await apiFetch<Response>('/api/gateways', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
Expand All @@ -653,6 +650,7 @@ function AddGatewayForm({ onAdded, onCancel }: { onAdded: () => void; onCancel:
token: form.token,
is_primary: false,
}),
raw: true,
})
const data = await res.json()
if (!res.ok) {
Expand Down
114 changes: 114 additions & 0 deletions src/lib/__tests__/api-client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
import { apiFetch, ApiError } from '../api-client'

const realFetch = global.fetch
const realLocation = window.location

function mockResponse(status: number, body: unknown = {}, opts: { json?: boolean } = { json: true }) {
return new Response(opts.json ? JSON.stringify(body) : String(body), {
status,
headers: { 'Content-Type': 'application/json' },
})
}

describe('apiFetch — global 401 / 403 / 5xx / network handling', () => {
let dispatched: CustomEvent[] = []
let originalHref = ''
let authExpiredListener: ((e: Event) => void) | null = null

beforeEach(() => {
dispatched = []
authExpiredListener = (e) => dispatched.push(e as CustomEvent)
window.addEventListener('mc:auth-expired', authExpiredListener)

// Stub location so redirect-to-login is observable
originalHref = window.location.href
Object.defineProperty(window, 'location', {
writable: true,
value: {
...realLocation,
pathname: '/cost-tracker',
search: '',
href: 'http://127.0.0.1:3000/cost-tracker',
},
})
})

afterEach(() => {
if (authExpiredListener) {
window.removeEventListener('mc:auth-expired', authExpiredListener)
authExpiredListener = null
}
global.fetch = realFetch
Object.defineProperty(window, 'location', { writable: true, value: realLocation })
window.location.href = originalHref
vi.restoreAllMocks()
})

it('returns parsed JSON on 200', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(200, { ok: true, count: 42 }))
const data = await apiFetch<{ ok: boolean; count: number }>('/api/tokens')
expect(data).toEqual({ ok: true, count: 42 })
})

it('emits mc:auth-expired and redirects to /login on 401', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'UNAUTHENTICATED',
status: 401,
})
expect(dispatched).toHaveLength(1)
expect(dispatched[0].detail).toMatchObject({ path: '/api/tokens', status: 401 })
expect(window.location.href).toContain('/login?from=%2Fcost-tracker')
})

it('does NOT redirect when redirectOnUnauthenticated=false', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' }))
await expect(
apiFetch('/api/tokens', { redirectOnUnauthenticated: false })
).rejects.toThrow(ApiError)
expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker')
})

it('throws FORBIDDEN on 403 without redirecting', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(403, { error: 'Requires admin role' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'FORBIDDEN',
status: 403,
})
expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker')
})

it('throws SERVER_ERROR on 500 with upstream message', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(500, { error: 'database is locked' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'SERVER_ERROR',
status: 500,
message: 'database is locked',
})
})

it('throws NETWORK_ERROR when fetch rejects', async () => {
global.fetch = vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'NETWORK_ERROR',
status: 0,
})
})

it('does not redirect when already on /login (avoid infinite loop)', async () => {
Object.defineProperty(window, 'location', {
writable: true,
value: { ...realLocation, pathname: '/login', search: '', href: 'http://127.0.0.1:3000/login' },
})
global.fetch = vi.fn().mockResolvedValue(mockResponse(401))
await expect(apiFetch('/api/auth/login', { method: 'POST' })).rejects.toThrow(ApiError)
expect(window.location.href).toBe('http://127.0.0.1:3000/login')
})

it('returns undefined for 204 No Content', async () => {
global.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 204 }))
const data = await apiFetch('/api/sessions/123', { method: 'DELETE' })
expect(data).toBeUndefined()
})
})
Loading