Frontend feature: add game detail page with contract event timeline#492
Merged
CMI-James merged 27 commits intotheblockcade:mainfrom Apr 3, 2026
Merged
Frontend feature: add game detail page with contract event timeline#492CMI-James merged 27 commits intotheblockcade:mainfrom
CMI-James merged 27 commits intotheblockcade:mainfrom
Conversation
…ith-timeline feat(frontend): add routable GameDetail page with contract event timeline
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a routable GameDetail page with event timeline, extends the typed API SDK with combined abort/timeout handling and a new getGameById method plus response type, migrates App to React Router v6 routing, updates backend OpenAPI and webhook docs, and includes new/updated tests and small import/typing cleanups. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as GameDetail (Browser)
participant SDK as ApiClient
participant API as Server
participant Feed as ContractEventFeed
Browser->>SDK: getGameById(gameId, { signal })
SDK->>API: GET /api/games/:gameId (uses combined AbortSignal)
API-->>SDK: 200 OK / 404 / error
SDK-->>Browser: ApiResult (success/error, data|null)
alt success & game present
Browser->>Feed: mount(contractId, autoStart=true, maxEvents=50)
Feed->>API: subscribe/start event stream
API-->>Feed: event messages
Feed-->>Browser: render event timeline
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/services/typed-api-sdk.ts`:
- Line 1: The file starts with a stray backtick before the JSDoc opener causing
a template literal to remain unclosed; open
frontend/src/services/typed-api-sdk.ts and remove the leading backtick so the
file begins with /** (fixing the broken template literal and restoring normal
TypeScript parsing for the module, including any exported
functions/classes/types in typed-api-sdk).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9025bb18-ad50-4b26-9c97-66a35ce06363
📒 Files selected for processing (1)
frontend/src/services/typed-api-sdk.ts
…ith-timeline-erkw31 Add GameDetail page, improve routing and ApiClient (abort/tracing), and add tests
There was a problem hiding this comment.
🧹 Nitpick comments (5)
frontend/tests/components/GameDetail.test.tsx (1)
5-7: Mock imports ApiClient but never uses it directly.The import of
ApiClienton line 5 is unused sincevi.mockhoists and replaces the module. While not harmful, this could be removed for clarity, or better yet, usevi.mockedfor type-safe mock access.Type-safe mock access pattern
-import { ApiClient } from '../../src/services/typed-api-sdk'; +import type { ApiClient } from '../../src/services/typed-api-sdk'; vi.mock('../../src/services/typed-api-sdk'); +const mockGetGameById = vi.fn(); +vi.mocked(ApiClient).prototype.getGameById = mockGetGameById; + // Then in tests: - (ApiClient as any).prototype.getGameById.mockResolvedValue({ + mockGetGameById.mockResolvedValue({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/tests/components/GameDetail.test.tsx` around lines 5 - 7, The test imports ApiClient but never uses it directly—remove the unused import or switch to a type-safe mock pattern by keeping the module mock (vi.mock('../../src/services/typed-api-sdk')) and using vi.mocked(ApiClient) where you need typed access; update GameDetail.test.tsx to either delete the ApiClient import or replace direct references with vi.mocked(ApiClient) so the mock is type-safe and the unused import is eliminated.frontend/src/App.tsx (1)
267-279: Consider using NavLink's built-inclassNamefunction.
NavLinksupportsclassName={({ isActive }) => isActive ? 'active' : ''}which handles active state automatically. However, your current approach may be intentional to group/gamesand/games/:gameIdunder the same active state. If that's the case, the manualactiveRoutelogic is appropriate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 267 - 279, NavLink components are manually using activeRoute to set 'active' instead of NavLink's built-in className function; update the NavLink props (in App.tsx where NavLink is used) to use className={({ isActive }) => isActive ? 'active' : ''} (and include end for exact-match routes like "/" by keeping end on the lobby NavLink) so active styling is handled automatically; if you intentionally want '/games' and '/games/:gameId' to share one active state, keep the manual activeRoute logic for the games NavLink and only replace the other NavLink usages with the isActive-based className function.frontend/src/services/typed-api-sdk.ts (1)
333-348: Minor:networkErr === "timeout"comparison is unreachable.When
controller.abort("timeout")is called,fetchthrows aDOMExceptionwithname: "AbortError", not the string"timeout". The timeout reason is only accessible viasignal.reason. The fallback checkrequestSignal?.reason === "timeout"already handles this correctly, making thenetworkErr === "timeout"comparison dead code.This doesn't affect correctness but could be simplified.
Suggested simplification
- const isAbort = networkErr?.name === "AbortError" || networkErr === "timeout"; + const isAbort = networkErr?.name === "AbortError"; const mappedNetErr = normalizeApiClientError( isAbort ? { code: - networkErr === "timeout" || requestSignal?.reason === "timeout" + requestSignal?.reason === "timeout" ? "API_REQUEST_TIMEOUT" : "API_ABORTED", domain: ErrorDomain.API, severity: ErrorSeverity.TERMINAL, message: - networkErr === "timeout" || requestSignal?.reason === "timeout" + requestSignal?.reason === "timeout" ? `Request timed out after ${opts.timeout}ms` : "Request was cancelled by the user.", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/services/typed-api-sdk.ts` around lines 333 - 348, The comparisons against networkErr === "timeout" are unreachable; when controller.abort("timeout") is used fetch throws a DOMException with name "AbortError" and the timeout reason is exposed via requestSignal?.reason. Update the logic around isAbort, mappedNetErr and the normalizeApiClientError call (symbols: networkErr, requestSignal, isAbort, normalizeApiClientError, mapRpcError, opts.timeout, ErrorDomain, ErrorSeverity) to only detect aborts by networkErr?.name === "AbortError" (or networkErr === "timeout" removed) and to check requestSignal?.reason === "timeout" for timeout-specific messaging/ codes ("API_REQUEST_TIMEOUT"); remove the dead networkErr === "timeout" branches so timeout handling relies solely on requestSignal?.reason.frontend/src/pages/GameDetail.tsx (2)
33-57: Consider using AbortController for request cancellation.The
activeflag prevents state updates after unmount but doesn't cancel the in-flight HTTP request. SincegetGameByIdacceptsopts.signal, you can abort the request on cleanup to save network resources on rapid navigation.AbortController integration
useEffect(() => { const normalizedGameId = gameId?.trim() ?? ''; if (!normalizedGameId) { setLoading(false); setError('Game id is required.'); setGame(null); return; } - let active = true; + const controller = new AbortController(); const loadGame = async () => { setLoading(true); setError(null); const client = new ApiClient(); - const result = await client.getGameById(normalizedGameId); + const result = await client.getGameById(normalizedGameId, { + signal: controller.signal, + }); - if (!active) return; + if (controller.signal.aborted) return; if (result.success) { setGame(result.data); } else { + // Don't set error state if request was aborted + if (result.error.code === 'API_ABORTED') return; setError(result.error.message); setGame(null); } setLoading(false); }; void loadGame(); return () => { - active = false; + controller.abort(); }; }, [gameId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/GameDetail.tsx` around lines 33 - 57, Replace the manual "active" flag with an AbortController: create const controller = new AbortController() in the component effect, pass controller.signal to ApiClient.getGameById (e.g., await client.getGameById(normalizedGameId, { signal: controller.signal }) or the appropriate opts object), and call controller.abort() in the cleanup; after the request resolves, only call setGame/setError/setLoading when !controller.signal.aborted (or handle the abort error in catch and skip state updates). Update references to loadGame and ApiClient.getGameById to use the controller.signal and remove the active flag logic.
9-16: Consider addingcontractId?: stringas an explicit property to theGameinterface.
game.contractIdcurrently relies on the catch-all index signature[key: string]: unknownrather than being declared as an explicit property. While the code defensively checks the type and uses a fallback, adding the property to the interface would improve type safety and make the API contract more explicit. Infrontend/src/types/api-client.ts, add:export interface Game { id: string; name: string; status: string; contractId?: string; [key: string]: unknown; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/GameDetail.tsx` around lines 9 - 16, Add an optional contractId?: string property to the Game interface to make game.contractId explicit and improve type safety; update the Game interface (the one currently declaring id, name, status and [key: string]: unknown) to include contractId?: string so resolveContractId(game: Game) no longer relies on the index signature and type-checks more precisely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/App.tsx`:
- Around line 267-279: NavLink components are manually using activeRoute to set
'active' instead of NavLink's built-in className function; update the NavLink
props (in App.tsx where NavLink is used) to use className={({ isActive }) =>
isActive ? 'active' : ''} (and include end for exact-match routes like "/" by
keeping end on the lobby NavLink) so active styling is handled automatically; if
you intentionally want '/games' and '/games/:gameId' to share one active state,
keep the manual activeRoute logic for the games NavLink and only replace the
other NavLink usages with the isActive-based className function.
In `@frontend/src/pages/GameDetail.tsx`:
- Around line 33-57: Replace the manual "active" flag with an AbortController:
create const controller = new AbortController() in the component effect, pass
controller.signal to ApiClient.getGameById (e.g., await
client.getGameById(normalizedGameId, { signal: controller.signal }) or the
appropriate opts object), and call controller.abort() in the cleanup; after the
request resolves, only call setGame/setError/setLoading when
!controller.signal.aborted (or handle the abort error in catch and skip state
updates). Update references to loadGame and ApiClient.getGameById to use the
controller.signal and remove the active flag logic.
- Around line 9-16: Add an optional contractId?: string property to the Game
interface to make game.contractId explicit and improve type safety; update the
Game interface (the one currently declaring id, name, status and [key: string]:
unknown) to include contractId?: string so resolveContractId(game: Game) no
longer relies on the index signature and type-checks more precisely.
In `@frontend/src/services/typed-api-sdk.ts`:
- Around line 333-348: The comparisons against networkErr === "timeout" are
unreachable; when controller.abort("timeout") is used fetch throws a
DOMException with name "AbortError" and the timeout reason is exposed via
requestSignal?.reason. Update the logic around isAbort, mappedNetErr and the
normalizeApiClientError call (symbols: networkErr, requestSignal, isAbort,
normalizeApiClientError, mapRpcError, opts.timeout, ErrorDomain, ErrorSeverity)
to only detect aborts by networkErr?.name === "AbortError" (or networkErr ===
"timeout" removed) and to check requestSignal?.reason === "timeout" for
timeout-specific messaging/ codes ("API_REQUEST_TIMEOUT"); remove the dead
networkErr === "timeout" branches so timeout handling relies solely on
requestSignal?.reason.
In `@frontend/tests/components/GameDetail.test.tsx`:
- Around line 5-7: The test imports ApiClient but never uses it directly—remove
the unused import or switch to a type-safe mock pattern by keeping the module
mock (vi.mock('../../src/services/typed-api-sdk')) and using
vi.mocked(ApiClient) where you need typed access; update GameDetail.test.tsx to
either delete the ApiClient import or replace direct references with
vi.mocked(ApiClient) so the mock is type-safe and the unused import is
eliminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70b2260a-24a0-41ae-bd8a-99b723083e4f
📒 Files selected for processing (9)
frontend/src/App.tsxfrontend/src/components/BreadCrumbs.tsxfrontend/src/pages/GameDetail.tsxfrontend/src/services/typed-api-sdk.tsfrontend/src/types/api-client.tsfrontend/tests/components/GameDetail.test.tsxfrontend/tests/components/v1/ExportCsvButton.test.tsxfrontend/tests/components/v1/MultiStepProgressIndicator.test.tsxfrontend/tests/components/v1/TxStatusPanel.test.tsx
💤 Files with no reviewable changes (1)
- frontend/tests/components/v1/ExportCsvButton.test.tsx
✅ Files skipped from review due to trivial changes (4)
- frontend/tests/components/v1/MultiStepProgressIndicator.test.tsx
- frontend/tests/components/v1/TxStatusPanel.test.tsx
- frontend/src/components/BreadCrumbs.tsx
- frontend/src/types/api-client.ts
…ith-timeline-wfj9zd Add GameDetail page and route, improve ApiClient request handling, and migrate App to React Router v6 patterns
…ortcsvbutton Fix frontend tests after error-boundary and CSV button updates
…amp-method Fix OpenAPI validation, document routes, and enable emergency-pause tests
Fix stellar integration failures when retry config is mocked
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/scripts/openapi.js (2)
266-279:⚠️ Potential issue | 🟠 MajorFail validation when a mounted router can't be resolved.
Line 273 now drops that mount from the generated spec and keeps going. If
routes/index.jsadds a router this parser can't map,backend/openapi.yamlquietly loses endpoints instead of surfacing the mismatch.🛠️ Suggested fix
const mounts = []; + const unresolvedMounts = []; let mountMatch; mountRegex.lastIndex = 0; while ((mountMatch = mountRegex.exec(source)) !== null) { const [, mountPath, variableName] = mountMatch; const filePath = variableToFile.get(variableName); if (!filePath) { - continue; + unresolvedMounts.push(`${mountPath} -> ${variableName}`); + continue; } mounts.push({ mountPath, filePath }); } + if (unresolvedMounts.length > 0) { + throw new Error(`Could not resolve mounted routers: ${unresolvedMounts.join(', ')}`); + } + return mounts;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/scripts/openapi.js` around lines 266 - 279, The parser currently silences unresolved mounted routers by skipping entries when filePath is missing; change the behavior in the mount extraction loop (the code using mountRegex.exec, mountPath, variableName and variableToFile) to fail fast instead of continuing: when variableToFile.get(variableName) returns falsy, throw a descriptive Error (including mountPath and variableName) so the OpenAPI generation/validation fails and surfaces the unresolved mount, rather than quietly dropping endpoints.
287-297:⚠️ Potential issue | 🟠 MajorTokenize handlers before checking for
auth.Line 294's regex also matches the string literal in
rateLimit('auth'). Inbackend/src/routes/users.routes.js:162, that makesPOST /createlook authenticated even though the route has noauthmiddleware, which is why/api/users/createis now emitted with bearer auth.🛠️ Suggested fix
while ((lineMatch = routeLineRegex.exec(source)) !== null) { const [, method, routePath, handlers] = lineMatch; + const handlerNames = handlers + .split(',') + .map((handler) => handler.trim().split('(')[0].trim()); routes.push({ method, path: routePath, - hasAuth: /\bauth\b/.test(handlers), - hasIdempotency: /\bidempotency\b/.test(handlers), - hasValidation: /\bvalidate\b/.test(handlers), + hasAuth: handlerNames.includes('auth'), + hasIdempotency: handlerNames.includes('idempotency'), + hasValidation: handlerNames.includes('validate'), }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/scripts/openapi.js` around lines 287 - 297, routeLineRegex currently detects "auth" inside quoted strings (e.g. rateLimit('auth')), causing false positives; when building the route object (the block using lineMatch, handlers, and routes.push) first strip string literals from handlers (e.g. remove /'[^']*'|"[^"]*"/ matches) then split the remaining text into tokens (split on non-word chars) and use those tokens to set hasAuth, hasIdempotency, and hasValidation (e.g. tokenArray.includes('auth')) so only actual middleware identifiers are detected.
🧹 Nitpick comments (1)
backend/src/services/stellar.service.js (1)
280-295: The syntheticNETWORK_ERRORfallback is misleading.Because Line 225 guarantees the loop runs at least once and the
catchpath always assignslastErrorbefore breaking, this branch is effectively dead on the non-throwing path. Keeping it around makes an internal/impossible failure look like a retryable network failure, which would change downstream handling inbackend/src/services/outbox.worker.jsat Lines 74-96. I'd either remove the fallback or use a distinct non-retryable internal/config error if you want to keep a safeguard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/services/stellar.service.js` around lines 280 - 295, The code returns a synthetic STELLAR_ERRORS.NETWORK_ERROR via resolvedError when lastError should always be set, which can misclassify an impossible internal state as retryable; change the fallback to a distinct non-retryable internal error (e.g., STELLAR_ERRORS.INTERNAL_ERROR or create one) and use a clear message like "Internal error: no error captured" so downstream logic (e.g., outbox worker) won't treat it as a network retry; update the resolvedError fallback and errorCode/errorMessage assignments in the block that builds the error response (refer to resolvedError, lastError, and STELLAR_ERRORS) so the fallback is non-retryable, or remove the fallback entirely if you prefer to assert lastError is always present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/routes/webhook.js`:
- Around line 28-35: The 401 response for the webhook route is using the shared
AuthErrorResponse (which only allows "No token provided" or "Invalid token");
replace that with a dedicated schema that matches the "Invalid webhook
signature" case: add a new OpenAPI component named WebhookSignatureErrorResponse
in your API spec (with the appropriate application/json schema allowing the
"Invalid webhook signature" message), and update the 401 response in the webhook
route to reference WebhookSignatureErrorResponse instead of AuthErrorResponse so
the contract is consistent.
In `@backend/src/services/stellar.service.js`:
- Around line 216-219: The current normalization silently sets malformed
maxRetries/retryInterval to 0, which disables retries; update the logic around
normalizedMaxRetries and normalizedRetryInterval to preserve the safe defaults
(use the config defaults of 3 and 1000 ms from the stellar config) instead of 0
when inputs are invalid, and emit a warning log that includes the bad values
(e.g., reference maxRetries, retryInterval, normalizedMaxRetries,
normalizedRetryInterval) so operators see the misconfiguration; alternatively,
perform validation earlier (during config loading) and throw/fail-fast on
invalid values—ensure the change touches the normalization block that assigns
normalizedMaxRetries and normalizedRetryInterval and calls the logger to report
the invalid inputs.
---
Outside diff comments:
In `@backend/scripts/openapi.js`:
- Around line 266-279: The parser currently silences unresolved mounted routers
by skipping entries when filePath is missing; change the behavior in the mount
extraction loop (the code using mountRegex.exec, mountPath, variableName and
variableToFile) to fail fast instead of continuing: when
variableToFile.get(variableName) returns falsy, throw a descriptive Error
(including mountPath and variableName) so the OpenAPI generation/validation
fails and surfaces the unresolved mount, rather than quietly dropping endpoints.
- Around line 287-297: routeLineRegex currently detects "auth" inside quoted
strings (e.g. rateLimit('auth')), causing false positives; when building the
route object (the block using lineMatch, handlers, and routes.push) first strip
string literals from handlers (e.g. remove /'[^']*'|"[^"]*"/ matches) then split
the remaining text into tokens (split on non-word chars) and use those tokens to
set hasAuth, hasIdempotency, and hasValidation (e.g.
tokenArray.includes('auth')) so only actual middleware identifiers are detected.
---
Nitpick comments:
In `@backend/src/services/stellar.service.js`:
- Around line 280-295: The code returns a synthetic STELLAR_ERRORS.NETWORK_ERROR
via resolvedError when lastError should always be set, which can misclassify an
impossible internal state as retryable; change the fallback to a distinct
non-retryable internal error (e.g., STELLAR_ERRORS.INTERNAL_ERROR or create one)
and use a clear message like "Internal error: no error captured" so downstream
logic (e.g., outbox worker) won't treat it as a network retry; update the
resolvedError fallback and errorCode/errorMessage assignments in the block that
builds the error response (refer to resolvedError, lastError, and
STELLAR_ERRORS) so the fallback is non-retryable, or remove the fallback
entirely if you prefer to assert lastError is always present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21c9ba9e-86d3-4211-a0de-033cc0ed517e
📒 Files selected for processing (6)
backend/openapi.yamlbackend/scripts/openapi.jsbackend/src/routes/users.routes.jsbackend/src/routes/webhook.jsbackend/src/services/stellar.service.jscontracts/emergency-pause/src/test.rs
✅ Files skipped from review due to trivial changes (1)
- backend/src/routes/users.routes.js
Comment on lines
+28
to
+35
| 401: { | ||
| description: 'Invalid webhook signature', | ||
| content: { | ||
| 'application/json': { | ||
| schema: { $ref: '#/components/schemas/AuthErrorResponse' }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Don't reuse AuthErrorResponse for signature failures.
This route advertises "Invalid webhook signature", but the shared schema in backend/scripts/openapi.js at Lines 54-63 only allows "No token provided" or "Invalid token". The new contract is internally inconsistent as written.
🛠️ Suggested fix
description: 'Invalid webhook signature',
content: {
'application/json': {
- schema: { $ref: '#/components/schemas/AuthErrorResponse' },
+ schema: {
+ type: 'object',
+ required: ['message'],
+ properties: {
+ message: { type: 'string', enum: ['Invalid webhook signature'] },
+ },
+ },
},
},
},If you want to keep this shared, add a dedicated WebhookSignatureErrorResponse component instead of inlining it here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 401: { | |
| description: 'Invalid webhook signature', | |
| content: { | |
| 'application/json': { | |
| schema: { $ref: '#/components/schemas/AuthErrorResponse' }, | |
| }, | |
| }, | |
| }, | |
| 401: { | |
| description: 'Invalid webhook signature', | |
| content: { | |
| 'application/json': { | |
| schema: { | |
| type: 'object', | |
| required: ['message'], | |
| properties: { | |
| message: { type: 'string', enum: ['Invalid webhook signature'] }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/routes/webhook.js` around lines 28 - 35, The 401 response for the
webhook route is using the shared AuthErrorResponse (which only allows "No token
provided" or "Invalid token"); replace that with a dedicated schema that matches
the "Invalid webhook signature" case: add a new OpenAPI component named
WebhookSignatureErrorResponse in your API spec (with the appropriate
application/json schema allowing the "Invalid webhook signature" message), and
update the 401 response in the webhook route to reference
WebhookSignatureErrorResponse instead of AuthErrorResponse so the contract is
consistent.
Comment on lines
+216
to
+219
| const normalizedMaxRetries = | ||
| Number.isInteger(maxRetries) && maxRetries >= 0 ? maxRetries : 0; | ||
| const normalizedRetryInterval = | ||
| Number.isFinite(retryInterval) && retryInterval >= 0 ? retryInterval : 0; |
There was a problem hiding this comment.
Don't silently downgrade malformed retry config to 0/0.
backend/src/config/stellar.js at Lines 46-47 already defaults these knobs to 3 retries and 1000 ms. Falling back to 0 here means an env typo disables retries entirely, and an invalid interval collapses retries into a tight loop. For a core submission path, that's a material resilience regression. Please either validate/fail fast in config loading or preserve the same safe defaults and log the bad values.
One way to preserve safe defaults here
- const normalizedMaxRetries =
- Number.isInteger(maxRetries) && maxRetries >= 0 ? maxRetries : 0;
- const normalizedRetryInterval =
- Number.isFinite(retryInterval) && retryInterval >= 0 ? retryInterval : 0;
+ const normalizedMaxRetries =
+ Number.isInteger(maxRetries) && maxRetries >= 0 ? maxRetries : 3;
+ const normalizedRetryInterval =
+ Number.isFinite(retryInterval) && retryInterval >= 0 ? retryInterval : 1000;
+
+ if (normalizedMaxRetries !== maxRetries || normalizedRetryInterval !== retryInterval) {
+ logger.warn('Invalid Stellar retry config; using defaults.', {
+ maxRetries,
+ retryInterval,
+ normalizedMaxRetries,
+ normalizedRetryInterval,
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/services/stellar.service.js` around lines 216 - 219, The current
normalization silently sets malformed maxRetries/retryInterval to 0, which
disables retries; update the logic around normalizedMaxRetries and
normalizedRetryInterval to preserve the safe defaults (use the config defaults
of 3 and 1000 ms from the stellar config) instead of 0 when inputs are invalid,
and emit a warning log that includes the bad values (e.g., reference maxRetries,
retryInterval, normalizedMaxRetries, normalizedRetryInterval) so operators see
the misconfiguration; alternatively, perform validation earlier (during config
loading) and throw/fail-fast on invalid values—ensure the change touches the
normalization block that assigns normalizedMaxRetries and
normalizedRetryInterval and calls the logger to report the invalid inputs.
…unctionality Regenerate contract docs to match current contract interfaces
Contributor
Author
|
All done @CMI-James |
…ntend Fix frontend typecheck and restore typed API client contract
…ation-sync-issue Regenerate contract documentation with updated generator output
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
frontend/src/pages/GameDetail.tsxwhich reads:gameIdfrom the route, fetches the game viaApiClient.getGameById, and renders loading, error, empty and summary states while wiringContractEventFeedfor the timeline.frontend/src/App.tsxto usereact-router-domRoutes/Route, add/games/:gameId, and switch command palette/navigation actions to router navigation.GetGameByIdResponseandApiClient.getGameById(gameId)including input validation infrontend/src/types/api-client.tsandfrontend/src/services/typed-api-sdk.ts.frontend/tests/components/GameDetail.test.tsxcovering successful fetch + timeline render, deterministic error state, and deterministic empty (null) response handling.closes Frontend feature: add game detail page with contract event timeline #417
Type of change
How Has This Been Tested?
Ran the unit tests for the new page:
cd frontend && npm run test -- GameDetail.test.tsx, and all tests passed (3 tests).cd frontend && npm run typecheck, which reported TypeScript issues that are pre-existing in unrelated files and therefore remain outside this feature scope; the feature changes themselves are type-annotated and the new tests run green.Checklist:
Summary by CodeRabbit
New Features
Refactor
Tests
Chores