Settings page overhaul, auth resilience, and automated testing #38

Merged
uz1mani merged 21 commits from staging into main 2026-03-01 13:05:56 +00:00
9 changed files with 169 additions and 17 deletions
Showing only changes of commit 22bc18a7cc - Show all commits

View File

@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
### Changed
- **Request ID tracing for debugging.** All API requests now include a unique Request ID header (`X-Request-ID`) that helps trace requests across frontend and backend services. When errors occur, the Request ID is included in the response, making it easy to find the exact request in server logs for debugging.
- **App Switcher now shows consistent order.** The Ciphera Apps menu now always displays apps in the same order: Pulse, Drop, Auth — regardless of which app you're currently using. Previously, the current app was shown first, causing the order to change depending on context. This creates a more predictable navigation experience.
### Fixed

View File

@@ -1,8 +1,10 @@
/**
* HTTP client wrapper for API calls
* Includes Request ID propagation for debugging across services
*/
import { authMessageFromStatus, AUTH_ERROR_MESSAGES } from '@ciphera-net/ui'
import { generateRequestId, getRequestIdHeader, setLastRequestId } from '@/lib/utils/requestId'
/** Request timeout in ms; network errors surface as user-facing "Network error, please try again." */
const FETCH_TIMEOUT_MS = 30_000
@@ -180,8 +182,13 @@ async function apiRequest<T>(
? `${baseUrl}${endpoint}`
: `${baseUrl}/api/v1${endpoint}`
// * Generate and store request ID for tracing
const requestId = generateRequestId()
setLastRequestId(requestId)
const headers: Record<string, string> = {
'Content-Type': 'application/json',
[getRequestIdHeader()]: requestId,
}
// * Merge any additional headers from options

79
lib/utils/errorHandler.ts Normal file
View File

@@ -0,0 +1,79 @@
/**
* Error handling utilities with Request ID extraction
* Helps users report errors with traceable IDs for support
*/
import { getLastRequestId } from './requestId'
interface ApiErrorResponse {
error?: {
code?: string
message?: string
details?: unknown
request_id?: string
}
}
/**
* Extract request ID from error response or use last known request ID
*/
export function getRequestIdFromError(errorData?: ApiErrorResponse): string | null {
// * Try to get from error response body
if (errorData?.error?.request_id) {
return errorData.error.request_id
}
// * Fallback to last request ID stored during API call
return getLastRequestId()
}
/**
* Format error message for display with optional request ID
* Shows request ID in development or for specific error types
*/
export function formatErrorMessage(
message: string,
errorData?: ApiErrorResponse,
options: { showRequestId?: boolean } = {}
): string {
const requestId = getRequestIdFromError(errorData)
// * Always show request ID in development
const isDev = process.env.NODE_ENV === 'development'
if (requestId && (isDev || options.showRequestId)) {
return `${message}\n\nRequest ID: ${requestId}`
}
return message
}
/**
* Log error with request ID for debugging
*/
export function logErrorWithRequestId(
context: string,
error: unknown,
errorData?: ApiErrorResponse
): void {
const requestId = getRequestIdFromError(errorData)
if (requestId) {
console.error(`[${context}] Request ID: ${requestId}`, error)
} else {
console.error(`[${context}]`, error)
}
}
/**
* Get support message with request ID for user reports
*/
export function getSupportMessage(errorData?: ApiErrorResponse): string {
const requestId = getRequestIdFromError(errorData)
if (requestId) {
return `If this persists, contact support with Request ID: ${requestId}`
}
return 'If this persists, please contact support.'
}

43
lib/utils/requestId.ts Normal file
View File

@@ -0,0 +1,43 @@
/**
* Request ID utilities for tracing API calls across services
* Request IDs help debug issues by correlating logs across frontend and backends
*/
const REQUEST_ID_HEADER = 'X-Request-ID'
/**
* Generate a unique request ID
* Format: REQ<timestamp>_<random>
*/
export function generateRequestId(): string {
const timestamp = Date.now().toString(36)
const random = Math.random().toString(36).substring(2, 8)
return `REQ${timestamp}_${random}`
}
/**
* Get request ID header name
*/
export function getRequestIdHeader(): string {
return REQUEST_ID_HEADER
}
/**
* Store the last request ID for error reporting
*/
let lastRequestId: string | null = null
export function setLastRequestId(id: string): void {
lastRequestId = id
}
export function getLastRequestId(): string | null {
return lastRequestId
}
/**
* Clear the stored request ID
*/
export function clearLastRequestId(): void {
lastRequestId = null
greptile-apps[bot] commented 2026-02-28 23:45:36 +00:00 (Migrated from github.com)
Review

Module-level singleton is unsafe for concurrent server-side use

lastRequestId is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If apiRequest is called simultaneously from two server-side contexts, request A sets lastRequestId = "REQ_A", then request B sets it to "REQ_B", and request A's error handler ends up reporting "REQ_B" — attributing the wrong request ID to the error.

Today apiRequest is primarily called from 'use client' components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from generateRequestId and passing it explicitly through error paths, or by using an AsyncLocalStorage context for server-side scenarios.

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/utils/requestId.ts
Line: 28-36

Comment:
**Module-level singleton is unsafe for concurrent server-side use**

`lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error.

Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios.

How can I resolve this? If you propose a fix, please make it concise.
**Module-level singleton is unsafe for concurrent server-side use** `lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error. Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: lib/utils/requestId.ts Line: 28-36 Comment: **Module-level singleton is unsafe for concurrent server-side use** `lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error. Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-03-01 12:52:51 +00:00 (Migrated from github.com)
Review

Issue: lastRequestId is a module-level mutable variable. In a Node.js SSR context, concurrent requests could overwrite each other's request IDs.
Fix: Added documentation explicitly scoping this module to client-side use only. apiRequest (the sole caller of setLastRequestId) runs exclusively in the browser where JS is single-threaded, so no concurrent mutation can occur.
Why: The practical risk is zero today, but the intent constraint should be documented so future server-side callers know to use a different pattern (e.g. AsyncLocalStorage).

Issue: `lastRequestId` is a module-level mutable variable. In a Node.js SSR context, concurrent requests could overwrite each other's request IDs. Fix: Added documentation explicitly scoping this module to client-side use only. `apiRequest` (the sole caller of `setLastRequestId`) runs exclusively in the browser where JS is single-threaded, so no concurrent mutation can occur. Why: The practical risk is zero today, but the intent constraint should be documented so future server-side callers know to use a different pattern (e.g. `AsyncLocalStorage`).
}