Add typed API client with auth and error handling (F0-005)
- Create ApiError class with status helpers (isNotFound, etc.) - Create typed fetch wrapper with get/post/put/patch/delete methods - Auto-inject Authorization header from auth store - Handle 401 with automatic token refresh and retry - Add query parameter support and proper URL building - Add comprehensive tests (11 tests) Phase F0 progress: 6/8 tasks complete Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
f63e8be600
commit
0720084cb1
@ -6,7 +6,7 @@
|
||||
"created": "2026-01-30",
|
||||
"lastUpdated": "2026-01-30",
|
||||
"totalTasks": 8,
|
||||
"completedTasks": 5,
|
||||
"completedTasks": 6,
|
||||
"status": "in_progress"
|
||||
},
|
||||
"tasks": [
|
||||
@ -101,8 +101,8 @@
|
||||
"description": "HTTP client with auth token injection and refresh",
|
||||
"category": "api",
|
||||
"priority": 5,
|
||||
"completed": false,
|
||||
"tested": false,
|
||||
"completed": true,
|
||||
"tested": true,
|
||||
"dependencies": ["F0-001", "F0-004"],
|
||||
"files": [
|
||||
{"path": "src/api/client.ts", "status": "create"},
|
||||
|
||||
304
frontend/src/api/client.spec.ts
Normal file
304
frontend/src/api/client.spec.ts
Normal file
@ -0,0 +1,304 @@
|
||||
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'
|
||||
import { setActivePinia, createPinia } from 'pinia'
|
||||
|
||||
import { useAuthStore } from '@/stores/auth'
|
||||
import { apiClient } from './client'
|
||||
import { ApiError } from './types'
|
||||
|
||||
// Mock fetch globally
|
||||
const mockFetch = vi.fn()
|
||||
vi.stubGlobal('fetch', mockFetch)
|
||||
|
||||
describe('apiClient', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createPinia())
|
||||
mockFetch.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
describe('get', () => {
|
||||
it('makes GET request to correct URL', async () => {
|
||||
/**
|
||||
* Test that GET requests are sent to the correct URL.
|
||||
*
|
||||
* The API client must correctly build URLs from the base URL
|
||||
* and path, which is fundamental to all API operations.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({ id: '1', name: 'Test' }),
|
||||
})
|
||||
|
||||
const result = await apiClient.get('/api/users/me')
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
expect.stringContaining('/api/users/me'),
|
||||
expect.objectContaining({ method: 'GET' })
|
||||
)
|
||||
expect(result).toEqual({ id: '1', name: 'Test' })
|
||||
})
|
||||
|
||||
it('adds query parameters to URL', async () => {
|
||||
/**
|
||||
* Test that query parameters are correctly appended.
|
||||
*
|
||||
* Many API endpoints accept query parameters for filtering,
|
||||
* pagination, etc. These must be properly encoded.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({ items: [] }),
|
||||
})
|
||||
|
||||
await apiClient.get('/api/cards', { params: { page: 1, limit: 20 } })
|
||||
|
||||
const calledUrl = mockFetch.mock.calls[0][0] as string
|
||||
expect(calledUrl).toContain('page=1')
|
||||
expect(calledUrl).toContain('limit=20')
|
||||
})
|
||||
|
||||
it('skips undefined query parameters', async () => {
|
||||
/**
|
||||
* Test that undefined parameters are not included in URL.
|
||||
*
|
||||
* Optional parameters should be omitted entirely rather than
|
||||
* being sent as "undefined" strings.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({}),
|
||||
})
|
||||
|
||||
await apiClient.get('/api/cards', {
|
||||
params: { page: 1, search: undefined },
|
||||
})
|
||||
|
||||
const calledUrl = mockFetch.mock.calls[0][0] as string
|
||||
expect(calledUrl).toContain('page=1')
|
||||
expect(calledUrl).not.toContain('search')
|
||||
})
|
||||
})
|
||||
|
||||
describe('post', () => {
|
||||
it('makes POST request with JSON body', async () => {
|
||||
/**
|
||||
* Test that POST requests include JSON body.
|
||||
*
|
||||
* POST requests typically send data to create resources.
|
||||
* The body must be JSON serialized.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 201,
|
||||
json: async () => ({ id: '1' }),
|
||||
})
|
||||
|
||||
const body = { name: 'New Deck', cards: [] }
|
||||
await apiClient.post('/api/decks', body)
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.objectContaining({
|
||||
method: 'POST',
|
||||
body: JSON.stringify(body),
|
||||
})
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('authentication', () => {
|
||||
it('adds Authorization header when authenticated', async () => {
|
||||
/**
|
||||
* Test that auth header is automatically added.
|
||||
*
|
||||
* Authenticated requests must include the Bearer token
|
||||
* so the backend can identify the user.
|
||||
*/
|
||||
const auth = useAuthStore()
|
||||
auth.setTokens({
|
||||
accessToken: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
expiresAt: Date.now() + 3600000,
|
||||
})
|
||||
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({}),
|
||||
})
|
||||
|
||||
await apiClient.get('/api/users/me')
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.objectContaining({
|
||||
headers: expect.objectContaining({
|
||||
Authorization: 'Bearer test-token',
|
||||
}),
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('skips auth header when skipAuth is true', async () => {
|
||||
/**
|
||||
* Test that skipAuth option prevents auth header.
|
||||
*
|
||||
* Some endpoints (like login/register) don't need auth
|
||||
* and should not send the token.
|
||||
*/
|
||||
const auth = useAuthStore()
|
||||
auth.setTokens({
|
||||
accessToken: 'test-token',
|
||||
refreshToken: 'refresh-token',
|
||||
expiresAt: Date.now() + 3600000,
|
||||
})
|
||||
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({}),
|
||||
})
|
||||
|
||||
await apiClient.get('/api/public/health', { skipAuth: true })
|
||||
|
||||
const calledOptions = mockFetch.mock.calls[0][1] as RequestInit
|
||||
expect(calledOptions.headers).not.toHaveProperty('Authorization')
|
||||
})
|
||||
|
||||
it('does not add auth header when not authenticated', async () => {
|
||||
/**
|
||||
* Test that unauthenticated requests have no auth header.
|
||||
*
|
||||
* Without a token, there's nothing to send.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({}),
|
||||
})
|
||||
|
||||
await apiClient.get('/api/public/status')
|
||||
|
||||
const calledOptions = mockFetch.mock.calls[0][1] as RequestInit
|
||||
const headers = calledOptions.headers as Record<string, string>
|
||||
expect(headers['Authorization']).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('error handling', () => {
|
||||
it('throws ApiError on non-2xx response', async () => {
|
||||
/**
|
||||
* Test that API errors are properly thrown.
|
||||
*
|
||||
* Non-successful responses should throw ApiError with
|
||||
* status code and message for proper error handling.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 404,
|
||||
statusText: 'Not Found',
|
||||
json: async () => ({ detail: 'Deck not found' }),
|
||||
})
|
||||
|
||||
try {
|
||||
await apiClient.get('/api/decks/999')
|
||||
expect.fail('Should have thrown')
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(ApiError)
|
||||
const error = e as ApiError
|
||||
expect(error.status).toBe(404)
|
||||
expect(error.detail).toBe('Deck not found')
|
||||
expect(error.isNotFound).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('handles non-JSON error responses', async () => {
|
||||
/**
|
||||
* Test that non-JSON errors are handled gracefully.
|
||||
*
|
||||
* Some server errors may return HTML or plain text.
|
||||
* The client should still create a proper ApiError.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 500,
|
||||
statusText: 'Internal Server Error',
|
||||
json: async () => {
|
||||
throw new Error('Not JSON')
|
||||
},
|
||||
})
|
||||
|
||||
await expect(apiClient.get('/api/crash')).rejects.toThrow(ApiError)
|
||||
})
|
||||
|
||||
it('handles 204 No Content responses', async () => {
|
||||
/**
|
||||
* Test that 204 responses return undefined.
|
||||
*
|
||||
* DELETE and some PUT/PATCH endpoints return 204 with no body.
|
||||
* The client should handle this without trying to parse JSON.
|
||||
*/
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 204,
|
||||
})
|
||||
|
||||
const result = await apiClient.delete('/api/decks/1')
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('token refresh', () => {
|
||||
it('retries request after successful token refresh on 401', async () => {
|
||||
/**
|
||||
* Test that 401 triggers token refresh and retry.
|
||||
*
|
||||
* When the access token expires, the client should automatically
|
||||
* refresh it and retry the failed request transparently.
|
||||
*/
|
||||
const auth = useAuthStore()
|
||||
auth.setTokens({
|
||||
accessToken: 'expired-token',
|
||||
refreshToken: 'valid-refresh-token',
|
||||
expiresAt: Date.now() + 3600000,
|
||||
})
|
||||
|
||||
// First call returns 401
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 401,
|
||||
statusText: 'Unauthorized',
|
||||
json: async () => ({ detail: 'Token expired' }),
|
||||
})
|
||||
|
||||
// Refresh token call succeeds
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
access_token: 'new-token',
|
||||
expires_in: 3600,
|
||||
}),
|
||||
})
|
||||
|
||||
// Retry succeeds
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({ id: '1', name: 'Test User' }),
|
||||
})
|
||||
|
||||
const result = await apiClient.get('/api/users/me')
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledTimes(3)
|
||||
expect(result).toEqual({ id: '1', name: 'Test User' })
|
||||
})
|
||||
})
|
||||
})
|
||||
177
frontend/src/api/client.ts
Normal file
177
frontend/src/api/client.ts
Normal file
@ -0,0 +1,177 @@
|
||||
/**
|
||||
* HTTP API client with authentication and error handling.
|
||||
*
|
||||
* Provides typed fetch wrapper that automatically:
|
||||
* - Injects Authorization header from auth store
|
||||
* - Refreshes tokens on 401 responses
|
||||
* - Parses JSON responses
|
||||
* - Throws typed ApiError on failures
|
||||
*/
|
||||
import { config } from '@/config'
|
||||
import { useAuthStore } from '@/stores/auth'
|
||||
import { ApiError } from './types'
|
||||
import type { RequestOptions, ErrorResponse } from './types'
|
||||
|
||||
/**
|
||||
* Build URL with query parameters.
|
||||
*/
|
||||
function buildUrl(path: string, params?: Record<string, string | number | boolean | undefined>): string {
|
||||
const base = config.apiBaseUrl.replace(/\/$/, '')
|
||||
const cleanPath = path.startsWith('/') ? path : `/${path}`
|
||||
const url = new URL(`${base}${cleanPath}`)
|
||||
|
||||
if (params) {
|
||||
Object.entries(params).forEach(([key, value]) => {
|
||||
if (value !== undefined) {
|
||||
url.searchParams.set(key, String(value))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
return url.toString()
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse error response from backend.
|
||||
*/
|
||||
async function parseErrorResponse(response: Response): Promise<ApiError> {
|
||||
let detail: string | undefined
|
||||
let code: string | undefined
|
||||
|
||||
try {
|
||||
const data: ErrorResponse = await response.json()
|
||||
detail = data.detail || data.message
|
||||
code = data.code
|
||||
} catch {
|
||||
// Response body is not JSON or empty
|
||||
}
|
||||
|
||||
return new ApiError(response.status, response.statusText, detail, code)
|
||||
}
|
||||
|
||||
/**
|
||||
* Make an authenticated API request.
|
||||
*
|
||||
* @param method - HTTP method
|
||||
* @param path - API path (e.g., '/api/users/me')
|
||||
* @param options - Request options
|
||||
* @returns Parsed JSON response
|
||||
* @throws ApiError on non-2xx responses
|
||||
*/
|
||||
async function request<T>(
|
||||
method: string,
|
||||
path: string,
|
||||
options: RequestOptions = {}
|
||||
): Promise<T> {
|
||||
const { skipAuth = false, headers = {}, body, params, signal } = options
|
||||
const auth = useAuthStore()
|
||||
|
||||
// Build headers
|
||||
const requestHeaders: Record<string, string> = {
|
||||
'Content-Type': 'application/json',
|
||||
...headers,
|
||||
}
|
||||
|
||||
// Add auth header if authenticated and not skipped
|
||||
if (!skipAuth && auth.isAuthenticated) {
|
||||
const token = await auth.getValidToken()
|
||||
if (token) {
|
||||
requestHeaders['Authorization'] = `Bearer ${token}`
|
||||
}
|
||||
}
|
||||
|
||||
// Make request
|
||||
const url = buildUrl(path, params)
|
||||
const response = await fetch(url, {
|
||||
method,
|
||||
headers: requestHeaders,
|
||||
body: body !== undefined ? JSON.stringify(body) : undefined,
|
||||
signal,
|
||||
})
|
||||
|
||||
// Handle 401 - try to refresh and retry once
|
||||
if (response.status === 401 && !skipAuth && auth.refreshToken) {
|
||||
const refreshed = await auth.refreshAccessToken()
|
||||
if (refreshed) {
|
||||
// Retry with new token
|
||||
const newToken = await auth.getValidToken()
|
||||
if (newToken) {
|
||||
requestHeaders['Authorization'] = `Bearer ${newToken}`
|
||||
const retryResponse = await fetch(url, {
|
||||
method,
|
||||
headers: requestHeaders,
|
||||
body: body !== undefined ? JSON.stringify(body) : undefined,
|
||||
signal,
|
||||
})
|
||||
|
||||
if (!retryResponse.ok) {
|
||||
throw await parseErrorResponse(retryResponse)
|
||||
}
|
||||
|
||||
// Handle 204 No Content
|
||||
if (retryResponse.status === 204) {
|
||||
return undefined as T
|
||||
}
|
||||
|
||||
return retryResponse.json()
|
||||
}
|
||||
}
|
||||
|
||||
// Refresh failed, throw the original 401
|
||||
throw await parseErrorResponse(response)
|
||||
}
|
||||
|
||||
// Handle other errors
|
||||
if (!response.ok) {
|
||||
throw await parseErrorResponse(response)
|
||||
}
|
||||
|
||||
// Handle 204 No Content
|
||||
if (response.status === 204) {
|
||||
return undefined as T
|
||||
}
|
||||
|
||||
return response.json()
|
||||
}
|
||||
|
||||
/**
|
||||
* API client with typed methods for each HTTP verb.
|
||||
*/
|
||||
export const apiClient = {
|
||||
/**
|
||||
* Make a GET request.
|
||||
*/
|
||||
get<T>(path: string, options?: RequestOptions): Promise<T> {
|
||||
return request<T>('GET', path, options)
|
||||
},
|
||||
|
||||
/**
|
||||
* Make a POST request.
|
||||
*/
|
||||
post<T>(path: string, body?: unknown, options?: RequestOptions): Promise<T> {
|
||||
return request<T>('POST', path, { ...options, body })
|
||||
},
|
||||
|
||||
/**
|
||||
* Make a PUT request.
|
||||
*/
|
||||
put<T>(path: string, body?: unknown, options?: RequestOptions): Promise<T> {
|
||||
return request<T>('PUT', path, { ...options, body })
|
||||
},
|
||||
|
||||
/**
|
||||
* Make a PATCH request.
|
||||
*/
|
||||
patch<T>(path: string, body?: unknown, options?: RequestOptions): Promise<T> {
|
||||
return request<T>('PATCH', path, { ...options, body })
|
||||
},
|
||||
|
||||
/**
|
||||
* Make a DELETE request.
|
||||
*/
|
||||
delete<T>(path: string, options?: RequestOptions): Promise<T> {
|
||||
return request<T>('DELETE', path, options)
|
||||
},
|
||||
}
|
||||
|
||||
export default apiClient
|
||||
11
frontend/src/api/index.ts
Normal file
11
frontend/src/api/index.ts
Normal file
@ -0,0 +1,11 @@
|
||||
/**
|
||||
* API module exports.
|
||||
*/
|
||||
export { apiClient, default } from './client'
|
||||
export { ApiError } from './types'
|
||||
export type {
|
||||
RequestOptions,
|
||||
ApiResponse,
|
||||
PaginatedResponse,
|
||||
ErrorResponse,
|
||||
} from './types'
|
||||
100
frontend/src/api/types.ts
Normal file
100
frontend/src/api/types.ts
Normal file
@ -0,0 +1,100 @@
|
||||
/**
|
||||
* API client types and error classes.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Custom error class for API errors.
|
||||
*
|
||||
* Provides structured error information including HTTP status code,
|
||||
* error message, and optional detail from the backend.
|
||||
*/
|
||||
export class ApiError extends Error {
|
||||
constructor(
|
||||
public readonly status: number,
|
||||
public readonly statusText: string,
|
||||
public readonly detail?: string,
|
||||
public readonly code?: string
|
||||
) {
|
||||
super(detail || statusText)
|
||||
this.name = 'ApiError'
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this is an authentication error (401).
|
||||
*/
|
||||
get isUnauthorized(): boolean {
|
||||
return this.status === 401
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this is a forbidden error (403).
|
||||
*/
|
||||
get isForbidden(): boolean {
|
||||
return this.status === 403
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this is a not found error (404).
|
||||
*/
|
||||
get isNotFound(): boolean {
|
||||
return this.status === 404
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this is a validation error (422).
|
||||
*/
|
||||
get isValidationError(): boolean {
|
||||
return this.status === 422
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this is a server error (5xx).
|
||||
*/
|
||||
get isServerError(): boolean {
|
||||
return this.status >= 500
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for API requests.
|
||||
*/
|
||||
export interface RequestOptions {
|
||||
/** Skip automatic auth header injection */
|
||||
skipAuth?: boolean
|
||||
/** Custom headers to merge with defaults */
|
||||
headers?: Record<string, string>
|
||||
/** Request body (will be JSON serialized) */
|
||||
body?: unknown
|
||||
/** Query parameters */
|
||||
params?: Record<string, string | number | boolean | undefined>
|
||||
/** AbortSignal for request cancellation */
|
||||
signal?: AbortSignal
|
||||
}
|
||||
|
||||
/**
|
||||
* Standard API response wrapper from backend.
|
||||
*/
|
||||
export interface ApiResponse<T> {
|
||||
data: T
|
||||
message?: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Paginated response from backend.
|
||||
*/
|
||||
export interface PaginatedResponse<T> {
|
||||
items: T[]
|
||||
total: number
|
||||
page: number
|
||||
pageSize: number
|
||||
totalPages: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Backend error response shape.
|
||||
*/
|
||||
export interface ErrorResponse {
|
||||
detail?: string
|
||||
message?: string
|
||||
code?: string
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user