fix: surface https backend requirement in dashboard#6623
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the dashboard's robustness and user experience by addressing mixed-content issues that arise when the dashboard is deployed over HTTPS but attempts to connect to an HTTP backend. It introduces a comprehensive validation and URL resolution system, ensuring that API connections are secure and correctly formed. By centralizing this logic and integrating validation into critical UI flows, the changes prevent common configuration pitfalls and provide users with clear, actionable feedback, moving away from opaque network errors to more informative messages. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant improvement, centralizing API URL handling and validation in a new request.ts utility. This effectively addresses the browser mixed-content issue when the dashboard is served over HTTPS. The refactoring across various components to use these new helpers is clean and enhances maintainability. I've identified a high-severity issue where the new validation logic could be bypassed due to the api store's initialization logic. My review includes specific suggestions to rectify this, which will also simplify the application's startup flow.
| init() { | ||
| if (this.apiBaseUrl) { | ||
| axios.defaults.baseURL = this.apiBaseUrl; | ||
| this.apiBaseUrl = setApiBaseUrl(this.apiBaseUrl); | ||
| } | ||
| }, |
There was a problem hiding this comment.
With the proposed change to state initialization, this init() method becomes redundant. The apiBaseUrl is correctly set globally in main.ts before the store is even created, and the store's state will be correctly initialized from that global value. Calling setApiBaseUrl again here is unnecessary. This method and its calls in main.ts can be removed to simplify the app's startup logic.
| const { useApiStore } = await import("@/stores/api"); | ||
| const apiStore = useApiStore(pinia); | ||
| apiStore.setPresets(presets); | ||
| apiStore.init(); |
There was a problem hiding this comment.
Following the simplification of the apiStore initialization, this call to apiStore.init() is no longer necessary and should be removed. Please see my related comments in dashboard/src/stores/api.ts for the full context.
| const { useApiStore } = await import("@/stores/api"); | ||
| const apiStore = useApiStore(pinia); | ||
| apiStore.setPresets(presets); | ||
| apiStore.init(); |
There was a problem hiding this comment.
This call to apiStore.init() is also redundant and should be removed as part of the apiStore initialization simplification.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
dosubot
bot
added
the
size:L
dosubot
bot
added
the
area:webui
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
checkAndSaveBackendyou normalize before validation, but in the Settings and Login flows you pass the raw URL intogetApiBaseUrlValidationError; consider normalizing first in those flows as well so validation behavior (e.g., handling of trailing slashes or missing scheme) is consistent across all entry points. - The error message in
checkAndSaveBackendcurrently interpolates the entire error object (Failed to connect to backend: ${error}); usingerror.messageor extracting a more user-friendly Axios error description would avoid noisy[object Object]output and give users clearer feedback.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `checkAndSaveBackend` you normalize before validation, but in the Settings and Login flows you pass the raw URL into `getApiBaseUrlValidationError`; consider normalizing first in those flows as well so validation behavior (e.g., handling of trailing slashes or missing scheme) is consistent across all entry points.
- The error message in `checkAndSaveBackend` currently interpolates the entire error object (`Failed to connect to backend: ${error}`); using `error.message` or extracting a more user-friendly Axios error description would avoid noisy `[object Object]` output and give users clearer feedback.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/authentication/auth/LoginPage.vue" line_range="29-35" />
<code_context>
const newPresetUrl = ref("");
function saveApiUrl() {
+ const validationError = getApiBaseUrlValidationError(apiUrl.value);
+ if (validationError) {
+ toast.error(validationError);
+ return;
+ }
+
apiStore.setApiBaseUrl(apiUrl.value);
serverConfigDialog.value = false;
window.location.reload();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align API URL handling here with the new normalization/validation pattern used on the welcome page.
Here `saveApiUrl` validates and then persists `apiUrl.value` directly, while the welcome page does normalize → validate → `setApiBaseUrl` using the normalized URL. If `normalizeConfiguredApiBaseUrl` enforces key invariants (e.g., trailing slashes, scheme), consider using it here as well so validation and stored values stay consistent and callers can always rely on a normalized base URL regardless of where it was configured.
Suggested implementation:
```
function saveApiUrl() {
const normalizedApiUrl = normalizeConfiguredApiBaseUrl(apiUrl.value);
const validationError = getApiBaseUrlValidationError(normalizedApiUrl);
if (validationError) {
toast.error(validationError);
return;
}
apiStore.setApiBaseUrl(normalizedApiUrl);
serverConfigDialog.value = false;
window.location.reload();
```
1. Ensure `normalizeConfiguredApiBaseUrl` is imported in this file from the same module used on the welcome page (e.g., a shared API config/util file).
2. If the welcome page wraps `normalizeConfiguredApiBaseUrl` in any additional error handling (e.g., try/catch if it can throw), mirror that pattern here for consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const validationError = getApiBaseUrlValidationError(apiUrl.value); | ||
| if (validationError) { | ||
| toast.error(validationError); | ||
| return; | ||
| } | ||
|
|
||
| apiStore.setApiBaseUrl(apiUrl.value); |
There was a problem hiding this comment.
suggestion (bug_risk): Align API URL handling here with the new normalization/validation pattern used on the welcome page.
Here saveApiUrl validates and then persists apiUrl.value directly, while the welcome page does normalize → validate → setApiBaseUrl using the normalized URL. If normalizeConfiguredApiBaseUrl enforces key invariants (e.g., trailing slashes, scheme), consider using it here as well so validation and stored values stay consistent and callers can always rely on a normalized base URL regardless of where it was configured.
Suggested implementation:
function saveApiUrl() {
const normalizedApiUrl = normalizeConfiguredApiBaseUrl(apiUrl.value);
const validationError = getApiBaseUrlValidationError(normalizedApiUrl);
if (validationError) {
toast.error(validationError);
return;
}
apiStore.setApiBaseUrl(normalizedApiUrl);
serverConfigDialog.value = false;
window.location.reload();
- Ensure
normalizeConfiguredApiBaseUrlis imported in this file from the same module used on the welcome page (e.g., a shared API config/util file). - If the welcome page wraps
normalizeConfiguredApiBaseUrlin any additional error handling (e.g., try/catch if it can throw), mirror that pattern here for consistency.
1 participant
Motivation
AxiosError: Network Errorfailures because an HTTP backend is blocked by browser mixed-content when the page is served over HTTPS.Description
getApiBaseUrlValidationErrorand related base normalization helpers todashboard/src/utils/request.tsto detect and return an actionable validation message for the HTTPS-page + HTTP-backend mixed-content case.dashboard/src/main.tsto run the validation and avoid applying stored HTTP backends when the dashboard is served overhttps:, usingsetApiBaseUrlonly when valid.dashboard/src/views/WelcomePage.vue,dashboard/src/views/Settings.vue,dashboard/src/views/authentication/auth/LoginPage.vue), and show an explicit error instead of attempting the request.Testing
git diff --checkand it reported no issues.ruff format .which completed successfully.ruff check .which failed due to pre-existing repository-wide Ruff violations unrelated to this change.cd dashboard && bunx prettier --check src/utils/request.ts src/main.ts src/views/WelcomePage.vue src/views/Settings.vue src/views/authentication/auth/LoginPage.vuewhich passed formatting checks.cd dashboard && bunx vite build --debugwhich entered the build pipeline and produced Vite transform logs but did not finish within the environment time window (timed out in this session).Codex Task
Summary by Sourcery
Validate and normalize configured backend URLs across the dashboard to avoid broken connections and mixed-content issues, and improve server compatibility with language-aware clients.
Bug Fixes:
Enhancements: