Skip to content

fix(frontend): make channel settings robust against API failures and proxy channel requests#365

Closed
opj161 wants to merge 1 commit into
HKUDS:mainfrom
opj161:fix-channel-settings
Closed

fix(frontend): make channel settings robust against API failures and proxy channel requests#365
opj161 wants to merge 1 commit into
HKUDS:mainfrom
opj161:fix-channel-settings

Conversation

@opj161

@opj161 opj161 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Goal

Make the frontend channel settings page robust against API failures and proxy channel requests in Vite.

## Affected Areas
- `frontend/vite.config.ts` (Proxy settings)
- `frontend/src/pages/Settings.tsx` (Resilience during page-load)
- `frontend/src/lib/api.ts` (Diagnostics on API response)
- `frontend/src/__tests__/` (Regression tests)

## Out of Scope
Backend changes, live broker connectivity, or mandate enforcement changes.

## Test Plan
Ran regression tests including `viteProxy.test.ts`, `api.test.ts`, and `SettingsChannels.test.tsx`. All 6 related tests pass.

## Risk Notes
No live trading, secret, or broker risks. This is entirely contained to frontend UI handling and dev server proxy routing. 

## Rollback Path
A standard `git revert` of this PR is safe.

…proxy channel requests

Signed-off-by: opj161 <opj161@outlook.com>
Copilot AI review requested due to automatic review settings July 1, 2026 00:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves the frontend Settings experience when channel runtime endpoints are unavailable (e.g., Vite SPA fallback returning HTML) by hardening API response parsing, proxying /channels during development, and rendering the Settings page even when channel status fails.

Changes:

  • Proxy /channels API routes through the Vite dev server to avoid SPA HTML fallback responses.
  • Switch Settings page loading to Promise.allSettled and make the channels panel resilient to channel-status load failures.
  • Add diagnostics + regression tests for non-JSON API responses and for Settings rendering when channel status fails.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
frontend/vite.config.ts Proxies /channels to the backend during dev to prevent SPA fallback HTML responses.
frontend/src/pages/Settings.tsx Uses allSettled and renders channels section even when channel status fails; disables actions when unavailable.
frontend/src/lib/api.ts Throws a descriptive ApiError when a successful response is not JSON (content-type mismatch).
frontend/src/pages/tests/SettingsChannels.test.tsx Adds regression test ensuring Settings still renders when channel status API rejects.
frontend/src/lib/tests/api.test.ts Adds unit test verifying non-JSON responses are rejected with a descriptive ApiError.
frontend/src/tests/viteProxy.test.ts Adds test that Vite config contains expected proxy paths (including /channels).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 54 to +58
useEffect(() => {
let alive = true;
Promise.all([api.getLLMSettings(), api.getDataSourceSettings(), api.getChannelStatus()])
.then(([llmData, dataSourceData, channelData]) => {

Promise.allSettled([
api.getLLMSettings(),
Comment on lines +69 to +76
const message = llmResult.reason instanceof Error ? llmResult.reason.message : "Unknown error";
setSettingsLoadError(message);
if (isAuthRequiredError(llmResult.reason)) {
toast.error(message);
} else {
toast.error(`Failed to load LLM settings: ${message}`);
}
}
Comment on lines +133 to +135
apiMock.getChannelStatus.mockRejectedValue(
new Error('Expected JSON from /channels/status, got text/html: <!doctype html>'),
);
@warren618

Copy link
Copy Markdown
Collaborator

Thanks - this has landed on main via the maintainer integration stack, preserving your fix as commit b79d6a8 and adding a small cleanup/regression layer in 0e952ef. Closing this PR to avoid a duplicate merge.

@warren618 warren618 closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants