Skip to content

Commit 9ae405f

Browse files
committed
fix: address adversarial review findings in resilience tests
- Remove unused authApi import that breaks ESLint/CI - Fix unhandled promise rejections: attach rejection handlers before advancing fake timers in boardStore and captureStore slow-API tests - Rename misleading test name ReturnsDegradedResult -> PropagatesException for the OpenAI timeout test (it asserts ThrowAsync, not a degraded result) - Fix contradictory comment in timeout test to match actual behavior - Fix misleading comment about "structurally valid JWT" when the test uses a structurally invalid token string
1 parent 00579b0 commit 9ae405f

2 files changed

Lines changed: 17 additions & 18 deletions

File tree

backend/tests/Taskdeck.Application.Tests/Services/LlmProviderResilienceTests.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ await act.Should().NotThrowAsync(
141141
// ── OpenAI: Network Timeout ─────────────────────────────────────
142142

143143
[Fact]
144-
public async Task OpenAi_CompleteAsync_HttpClientThrowsTimeout_ReturnsDegradedResult()
144+
public async Task OpenAi_CompleteAsync_HttpClientThrowsTimeout_PropagatesException()
145145
{
146146
var settings = BuildOpenAiSettings();
147147
var handler = new StubHttpMessageHandler((_, _) =>
@@ -150,16 +150,12 @@ public async Task OpenAi_CompleteAsync_HttpClientThrowsTimeout_ReturnsDegradedRe
150150
var provider = new OpenAiLlmProvider(
151151
new HttpClient(handler), settings, logger);
152152

153-
// TaskCanceledException from HttpClient timeout is NOT OperationCanceledException
154-
// from a caller's CancellationToken — the provider should catch it and return degraded.
155-
// However, OperationCanceledException is re-thrown by the provider.
156-
// TaskCanceledException IS an OperationCanceledException, so the provider re-throws.
157-
// This is correct behavior — callers handle it at the HTTP/controller layer.
153+
// TaskCanceledException from HttpClient timeout is an OperationCanceledException.
154+
// The provider intentionally re-throws this exception so that the caller (e.g., the
155+
// controller) can handle the timeout appropriately (e.g., by returning 504 Gateway Timeout).
158156
var act = async () => await provider.CompleteAsync(new ChatCompletionRequest(
159157
[new ChatCompletionMessage("User", "create a card")]));
160158

161-
// The provider re-throws OperationCanceledException (parent of TaskCanceledException).
162-
// This is intentional — the controller layer catches it and returns an appropriate response.
163159
await act.Should().ThrowAsync<OperationCanceledException>(
164160
"timeout exceptions should propagate so the controller layer can handle them");
165161
}

frontend/taskdeck-web/src/tests/resilience/slowApiAndStorage.spec.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { useCaptureStore } from '../../store/captureStore'
1111
import { useSessionStore } from '../../store/sessionStore'
1212
import { boardsApi } from '../../api/boardsApi'
1313
import { captureApi } from '../../api/captureApi'
14-
import { authApi } from '../../api/authApi'
1514
import * as tokenStorage from '../../utils/tokenStorage'
1615

1716
// ─── global mocks ────────────────────────────────────────────────────────────
@@ -201,9 +200,12 @@ describe('boardStore — slow API response handling', () => {
201200
const fetchPromise = store.fetchBoards()
202201
expect(store.loading).toBe(true)
203202

204-
await vi.advanceTimersByTimeAsync(5000)
203+
// Attach rejection handler BEFORE advancing timers to avoid
204+
// unhandled promise rejection when the timer fires the throw.
205+
const rejectExpectation = expect(fetchPromise).rejects.toThrow()
205206

206-
await expect(fetchPromise).rejects.toThrow()
207+
await vi.advanceTimersByTimeAsync(5000)
208+
await rejectExpectation
207209

208210
expect(store.loading).toBe(false)
209211
expect(store.error).toBeTruthy()
@@ -233,13 +235,14 @@ describe('captureStore — slow API response handling', () => {
233235
const store = useCaptureStore()
234236
const createPromise = store.createItem({ text: 'Slow capture', boardId: null })
235237

236-
await vi.advanceTimersByTimeAsync(6000)
238+
// Attach rejection handler BEFORE advancing timers to avoid
239+
// unhandled promise rejection when the timer fires the throw.
240+
const settled = createPromise.catch(() => {
241+
// Expected failure -- handled here to prevent unhandled rejection
242+
})
237243

238-
try {
239-
await createPromise
240-
} catch {
241-
// Expected failure
242-
}
244+
await vi.advanceTimersByTimeAsync(6000)
245+
await settled
243246

244247
expect(toastMocks.error).toHaveBeenCalled()
245248
})
@@ -262,7 +265,7 @@ describe('sessionStore — localStorage corruption mid-session', () => {
262265
// test restoreSession which is the path that runs on app init.
263266

264267
// First, seed localStorage with a previously valid session.
265-
// Use a structurally valid but expired-like JWT.
268+
// Use a structurally INVALID token (not 3 base64url segments) to test cleanup.
266269
localStorage.setItem('taskdeck_token', 'not-a-valid-jwt')
267270
localStorage.setItem('taskdeck_session', JSON.stringify({
268271
userId: 'u1',

0 commit comments

Comments
 (0)