feat(dashboard): Restore configurable API Base URL support#7306
feat(dashboard): Restore configurable API Base URL support#7306LIghtJUNction wants to merge 3 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The
window.fetchwrapper only rewrites URLs wheninputis a string starting with/api; for consistency with axios and to avoid surprises in production, consider also handlingRequestobjects whoseurlstarts with/apiso those calls go throughresolveApiUrlas well. - You now duplicate token/locale header attachment in the global axios interceptor in
main.tsand in the custom axios instance inrequest.ts; centralizing this logic on the shared instance (and using it everywhere) would reduce the risk of divergence or missed headers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `window.fetch` wrapper only rewrites URLs when `input` is a string starting with `/api`; for consistency with axios and to avoid surprises in production, consider also handling `Request` objects whose `url` starts with `/api` so those calls go through `resolveApiUrl` as well.
- You now duplicate token/locale header attachment in the global axios interceptor in `main.ts` and in the custom axios instance in `request.ts`; centralizing this logic on the shared instance (and using it everywhere) would reduce the risk of divergence or missed headers.
## Individual Comments
### Comment 1
<location path="dashboard/src/main.ts" line_range="124-133" />
<code_context>
+ // Keep fetch() calls consistent with axios by automatically attaching the JWT.
+ // Some parts of the UI use fetch directly; without this, those requests will 401.
+ const _origFetch = window.fetch.bind(window);
+ window.fetch = (input: RequestInfo | URL, init?: RequestInit) => {
+ let url = input;
+ if (typeof input === "string" && input.startsWith("/api")) {
+ url = resolveApiUrl(input, getApiBaseUrl());
+ }
+
+ const token = localStorage.getItem("token");
+ const headers = new Headers(
+ init?.headers ||
+ (typeof input !== "string" && "headers" in input
</code_context>
<issue_to_address>
**issue (bug_risk):** Relative `/api` URLs in `Request` objects are not rewritten to the API base URL.
This wrapper only rewrites URLs when `input` is a string starting with `/api`. If a caller passes `new Request("/api/foo")`, `url` remains the original `Request`, so its relative `/api/...` URL is never resolved to the configured API base URL. Please also handle `Request` inputs (e.g., by checking `request.url` and cloning with a resolved URL) so `fetch` behaves consistently regardless of how it’s called.
</issue_to_address>
### Comment 2
<location path="dashboard/src/main.ts" line_range="148-157" />
<code_context>
+async function initApp() {
</code_context>
<issue_to_address>
**issue (bug_risk):** App initialization order may use an outdated API base URL before `config.json` is applied.
Because `initApp()` is not awaited before mounting, any early use of Axios or `resolveApiUrl` can see a `baseURL` derived only from `VITE_API_BASE`, with `config.json`/localStorage overrides applied later. If those overrides must apply to all requests, ensure `initApp()` completes before mounting the app (or before any code that uses the API client runs).
</issue_to_address>
### Comment 3
<location path="dashboard/src/main.ts" line_range="120-129" />
<code_context>
+ });
+ });
axios.interceptors.request.use((config) => {
- const token = localStorage.getItem('token');
+ const token = localStorage.getItem("token");
if (token) {
- config.headers['Authorization'] = `Bearer ${token}`;
+ config.headers["Authorization"] = `Bearer ${token}`;
}
- const locale = localStorage.getItem('astrbot-locale');
+ const locale = localStorage.getItem("astrbot-locale");
if (locale) {
- config.headers['Accept-Language'] = locale;
+ config.headers["Accept-Language"] = locale;
}
return config;
});
</code_context>
<issue_to_address>
**suggestion (bug_risk):** There are now two Axios instances with separate interceptors, which can diverge behavior.
In this file you configure interceptors on the default `axios` export, while `src/utils/request.ts` defines its own `service` instance with different interceptor logic (including URL normalization). Depending on which import callers use, requests may see different headers/URL handling. Consider consolidating on a single configured Axios instance and reusing its interceptors to prevent behavioral drift.
Suggested implementation:
```typescript
```
1. Ensure that all API calls in the app use the configured Axios instance from `src/utils/request.ts` (usually something like `import service from "@/utils/request";`) instead of importing the default `axios` from `axios`.
2. If `dashboard/src/main.ts` imports `axios` only to add these interceptors, remove that import to avoid an unused import.
3. If there are any remaining places importing `axios` directly for HTTP calls, consider switching them to use the shared `service` instance so that URL normalization, headers, and other shared behavior are consistent across the app.
</issue_to_address>
### Comment 4
<location path="dashboard/src/utils/request.ts" line_range="9" />
<code_context>
+ return ABSOLUTE_URL_PATTERN.test(value);
+}
+
+function stripTrailingSlashes(value: string): string {
+ return value.replace(/\/+$/, "");
+}
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the URL handling by collapsing multiple helper functions into a single path normalizer and relying more on the standard URL API for joining and WebSocket URL construction.
You can keep the current behavior but reduce cognitive load by collapsing some helpers and letting `URL` do more of the joining work.
### 1. Collapse path/base helpers into a single normalization function
`stripTrailingSlashes`, `ensureLeadingSlash`, `stripLeadingApiPrefix`, `baseEndsWithApi`, and `normalizePathForBase` are all part of the same concern. You can keep the `/api`-when-base-ends-with-`/api` behavior but expose it via a single function instead of a multi-step pipeline:
```ts
const ABSOLUTE_URL_PATTERN = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//;
function isAbsoluteUrl(value: string): boolean {
return ABSOLUTE_URL_PATTERN.test(value);
}
function normalizePathForBase(path: string, baseUrl = ""): string {
if (!path) return "/";
if (isAbsoluteUrl(path)) return path;
const normalizedBase = (baseUrl ?? "").trim().replace(/\/+$/, "");
// base ends with /api → strip leading /api from path
let normalizedPath = path.startsWith("/") ? path : `/${path}`;
if (normalizedBase.endsWith("/api")) {
normalizedPath = normalizedPath.replace(/^\/api(?=\/|$)/, "") || "/";
}
return normalizedPath;
}
```
This removes `stripTrailingSlashes`, `ensureLeadingSlash`, `stripLeadingApiPrefix`, and `baseEndsWithApi` without changing the effective behavior of `normalizePathForBase` or the interceptor.
### 2. Use `URL` in `resolveApiUrl` instead of `joinBaseAndPath`
`joinBaseAndPath` can be removed in favor of `new URL`, while preserving the same semantics:
```ts
function normalizeBaseUrl(baseUrl: string | null | undefined): string {
return (baseUrl ?? "").trim().replace(/\/+$/, "");
}
export function resolveApiUrl(
path: string,
baseUrl: string | null | undefined = getApiBaseUrl(),
): string {
const normalizedBaseUrl = normalizeBaseUrl(baseUrl);
const normalizedPath = normalizePathForBase(path, normalizedBaseUrl);
if (isAbsoluteUrl(normalizedPath) || !normalizedBaseUrl) {
return normalizedPath;
}
// URL handles slashes for us; keep base as-is but add a trailing slash
return new URL(
normalizedPath.replace(/^\/+/, ""),
`${normalizedBaseUrl}/`,
).toString();
}
```
This lets you delete `joinBaseAndPath` entirely, and moves more of the path-joining logic into the standard library.
### 3. Simplify `resolveWebSocketUrl`
You don’t need a second `new URL` base when `resolveApiUrl` already returns an absolute URL. You can still attach query params and swap protocol:
```ts
export function resolveWebSocketUrl(
path: string,
queryParams?: Record<string, string>,
): string {
const url = new URL(resolveApiUrl(path));
if (url.protocol === "https:") url.protocol = "wss:";
else if (url.protocol === "http:") url.protocol = "ws:";
if (queryParams) {
for (const [key, value] of Object.entries(queryParams)) {
url.searchParams.set(key, value);
}
}
return url.toString();
}
```
This keeps all existing behavior but removes indirection and makes the flow easier to follow.
</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 locale = localStorage.getItem('astrbot-locale'); | ||
| const locale = localStorage.getItem("astrbot-locale"); | ||
| if (locale) { | ||
| config.headers['Accept-Language'] = locale; | ||
| config.headers["Accept-Language"] = locale; | ||
| } | ||
| return config; | ||
| }); | ||
|
|
||
| // Keep fetch() calls consistent with axios by automatically attaching the JWT. | ||
| // Some parts of the UI use fetch directly; without this, those requests will 401. | ||
| const _origFetch = window.fetch.bind(window); | ||
| window.fetch = (input: RequestInfo | URL, init?: RequestInit) => { | ||
| const token = localStorage.getItem('token'); | ||
| if (!token) return _origFetch(input, init); | ||
|
|
||
| const headers = new Headers(init?.headers || (typeof input !== 'string' && 'headers' in input ? (input as Request).headers : undefined)); | ||
| if (!headers.has('Authorization')) { | ||
| headers.set('Authorization', `Bearer ${token}`); | ||
| // 1. 定义加载配置的函数 | ||
| async function loadAppConfig() { |
There was a problem hiding this comment.
issue (bug_risk): Relative /api URLs in Request objects are not rewritten to the API base URL.
This wrapper only rewrites URLs when input is a string starting with /api. If a caller passes new Request("/api/foo"), url remains the original Request, so its relative /api/... URL is never resolved to the configured API base URL. Please also handle Request inputs (e.g., by checking request.url and cloning with a resolved URL) so fetch behaves consistently regardless of how it’s called.
| async function initApp() { | ||
| const config = await loadAppConfig(); | ||
| const configApiUrl = config.apiBaseUrl || ""; | ||
| const envApiUrl = import.meta.env.VITE_API_BASE || ""; | ||
|
|
||
| const localApiUrl = localStorage.getItem("apiBaseUrl"); | ||
| const apiBaseUrl = | ||
| localApiUrl !== null ? localApiUrl : configApiUrl || envApiUrl; | ||
|
|
||
| if (apiBaseUrl) { |
There was a problem hiding this comment.
issue (bug_risk): App initialization order may use an outdated API base URL before config.json is applied.
Because initApp() is not awaited before mounting, any early use of Axios or resolveApiUrl can see a baseURL derived only from VITE_API_BASE, with config.json/localStorage overrides applied later. If those overrides must apply to all requests, ensure initApp() completes before mounting the app (or before any code that uses the API client runs).
| axios.interceptors.request.use((config) => { | ||
| const token = localStorage.getItem('token'); | ||
| const token = localStorage.getItem("token"); | ||
| if (token) { | ||
| config.headers['Authorization'] = `Bearer ${token}`; | ||
| config.headers["Authorization"] = `Bearer ${token}`; | ||
| } | ||
| const locale = localStorage.getItem('astrbot-locale'); | ||
| const locale = localStorage.getItem("astrbot-locale"); | ||
| if (locale) { | ||
| config.headers['Accept-Language'] = locale; | ||
| config.headers["Accept-Language"] = locale; | ||
| } | ||
| return config; |
There was a problem hiding this comment.
suggestion (bug_risk): There are now two Axios instances with separate interceptors, which can diverge behavior.
In this file you configure interceptors on the default axios export, while src/utils/request.ts defines its own service instance with different interceptor logic (including URL normalization). Depending on which import callers use, requests may see different headers/URL handling. Consider consolidating on a single configured Axios instance and reusing its interceptors to prevent behavioral drift.
Suggested implementation:
- Ensure that all API calls in the app use the configured Axios instance from
src/utils/request.ts(usually something likeimport service from "@/utils/request";) instead of importing the defaultaxiosfromaxios. - If
dashboard/src/main.tsimportsaxiosonly to add these interceptors, remove that import to avoid an unused import. - If there are any remaining places importing
axiosdirectly for HTTP calls, consider switching them to use the sharedserviceinstance so that URL normalization, headers, and other shared behavior are consistent across the app.
| return ABSOLUTE_URL_PATTERN.test(value); | ||
| } | ||
|
|
||
| function stripTrailingSlashes(value: string): string { |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the URL handling by collapsing multiple helper functions into a single path normalizer and relying more on the standard URL API for joining and WebSocket URL construction.
You can keep the current behavior but reduce cognitive load by collapsing some helpers and letting URL do more of the joining work.
1. Collapse path/base helpers into a single normalization function
stripTrailingSlashes, ensureLeadingSlash, stripLeadingApiPrefix, baseEndsWithApi, and normalizePathForBase are all part of the same concern. You can keep the /api-when-base-ends-with-/api behavior but expose it via a single function instead of a multi-step pipeline:
const ABSOLUTE_URL_PATTERN = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//;
function isAbsoluteUrl(value: string): boolean {
return ABSOLUTE_URL_PATTERN.test(value);
}
function normalizePathForBase(path: string, baseUrl = ""): string {
if (!path) return "/";
if (isAbsoluteUrl(path)) return path;
const normalizedBase = (baseUrl ?? "").trim().replace(/\/+$/, "");
// base ends with /api → strip leading /api from path
let normalizedPath = path.startsWith("/") ? path : `/${path}`;
if (normalizedBase.endsWith("/api")) {
normalizedPath = normalizedPath.replace(/^\/api(?=\/|$)/, "") || "/";
}
return normalizedPath;
}This removes stripTrailingSlashes, ensureLeadingSlash, stripLeadingApiPrefix, and baseEndsWithApi without changing the effective behavior of normalizePathForBase or the interceptor.
2. Use URL in resolveApiUrl instead of joinBaseAndPath
joinBaseAndPath can be removed in favor of new URL, while preserving the same semantics:
function normalizeBaseUrl(baseUrl: string | null | undefined): string {
return (baseUrl ?? "").trim().replace(/\/+$/, "");
}
export function resolveApiUrl(
path: string,
baseUrl: string | null | undefined = getApiBaseUrl(),
): string {
const normalizedBaseUrl = normalizeBaseUrl(baseUrl);
const normalizedPath = normalizePathForBase(path, normalizedBaseUrl);
if (isAbsoluteUrl(normalizedPath) || !normalizedBaseUrl) {
return normalizedPath;
}
// URL handles slashes for us; keep base as-is but add a trailing slash
return new URL(
normalizedPath.replace(/^\/+/, ""),
`${normalizedBaseUrl}/`,
).toString();
}This lets you delete joinBaseAndPath entirely, and moves more of the path-joining logic into the standard library.
3. Simplify resolveWebSocketUrl
You don’t need a second new URL base when resolveApiUrl already returns an absolute URL. You can still attach query params and swap protocol:
export function resolveWebSocketUrl(
path: string,
queryParams?: Record<string, string>,
): string {
const url = new URL(resolveApiUrl(path));
if (url.protocol === "https:") url.protocol = "wss:";
else if (url.protocol === "http:") url.protocol = "ws:";
if (queryParams) {
for (const [key, value] of Object.entries(queryParams)) {
url.searchParams.set(key, value);
}
}
return url.toString();
}This keeps all existing behavior but removes indirection and makes the flow easier to follow.
There was a problem hiding this comment.
Code Review
This pull request introduces a dynamic API base URL configuration mechanism, allowing the application to load settings from a 'config.json' file at runtime. It includes a new utility module for request handling and updates the main entry point to initialize these configurations. My review highlights a race condition in the initialization sequence, suggests removing redundant global Axios interceptors, recommends improving the fetch wrapper to support non-string inputs, and advises against exporting the entire Axios package to prevent accidental usage of the unconfigured global instance.
| } | ||
| }); | ||
| }); | ||
| setupI18n() |
There was a problem hiding this comment.
There is a race condition between initApp() and setupI18n(). initApp is an asynchronous function that fetches the dynamic configuration and sets up the fetch wrapper, but it is currently called at the end of the file without being awaited. This allows setupI18n and the application mounting process to begin before the API base URL is correctly initialized, which could lead to failed requests during the initial load.
| setupI18n() | |
| initApp().then(() => setupI18n()) |
| axios.interceptors.request.use((config) => { | ||
| const token = localStorage.getItem('token'); | ||
| const token = localStorage.getItem("token"); | ||
| if (token) { | ||
| config.headers['Authorization'] = `Bearer ${token}`; | ||
| config.headers["Authorization"] = `Bearer ${token}`; | ||
| } | ||
| const locale = localStorage.getItem('astrbot-locale'); | ||
| const locale = localStorage.getItem("astrbot-locale"); | ||
| if (locale) { | ||
| config.headers['Accept-Language'] = locale; | ||
| config.headers["Accept-Language"] = locale; | ||
| } | ||
| return config; | ||
| }); |
There was a problem hiding this comment.
This global Axios interceptor is redundant. The application uses a custom Axios instance (service) defined in @/utils/request.ts, which already implements token and locale injection in its own interceptor. Interceptors added to the global axios object do not affect instances created via axios.create(). Removing this block will simplify the code and avoid confusion.
| if (typeof input === "string" && input.startsWith("/api")) { | ||
| url = resolveApiUrl(input, getApiBaseUrl()); | ||
| } |
There was a problem hiding this comment.
The fetch wrapper currently only resolves URLs when the input is a string. If the application uses URL or Request objects for API calls, the dynamic API base URL resolution will be skipped. Consider updating this logic to handle URL and Request objects to ensure all fetch calls are correctly routed.
| }); | ||
|
|
||
| export default service; | ||
| export * from "axios"; |
There was a problem hiding this comment.
export * from "axios" exports all named members of the axios package, which operate on the global axios instance rather than the custom service instance defined here. This can lead to bugs if developers accidentally import and use the unconfigured global instance. It is safer to only export the service instance and the specific types needed by the application.
| export * from "axios"; | |
| export type { AxiosResponse, InternalAxiosRequestConfig, AxiosError } from "axios"; |
There was a problem hiding this comment.
Pull request overview
This PR aims to restore support for configuring the dashboard’s API Base URL (via VITE_API_BASE and/or a runtime config.json) so the frontend can be deployed independently from the backend.
Changes:
- Adds a new request utility (
src/utils/request.ts) to normalize and resolve API/public/WebSocket URLs and manage a dedicated axios instance base URL. - Updates
src/main.tsto loadconfig.json, apply a chosen API base URL, and patchwindow.fetchto align/api/*calls with the configured API base. - Updates
vite.config.tsSCSS preprocessor options to reduce Dart Sass deprecation noise.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dashboard/vite.config.ts | Adds SCSS preprocessor options intended to silence Sass deprecation warnings; minor formatting changes. |
| dashboard/src/utils/request.ts | Introduces API base URL normalization/resolution helpers and an axios instance with request normalization. |
| dashboard/src/main.ts | Loads runtime config, selects API base URL, sets it via request utils, and patches fetch for /api/* calls. |
| if (apiBaseUrl) { | ||
| console.log(`API Base URL set to: ${apiBaseUrl}`); | ||
| } | ||
| return _origFetch(input, { ...init, headers }); | ||
| }; | ||
|
|
||
| loader.config({ monaco }) | ||
| setApiBaseUrl(apiBaseUrl); |
There was a problem hiding this comment.
setApiBaseUrl() only updates the custom axios instance in @/utils/request, but most of the dashboard code imports and uses the default axios instance directly (e.g. many axios.get('/api/...') calls). As a result, changing the API Base URL here won’t actually affect the majority of API requests, and /api will still be resolved against the dashboard origin.
Consider either (1) configuring the global axios.defaults.baseURL + a request interceptor to rewrite /api/... paths based on the configured base, or (2) migrating call sites to use the exported request service instance consistently.
| async function loadAppConfig() { | ||
| try { | ||
| const configUrl = new URL(resolvePublicUrl("config.json")); | ||
| configUrl.searchParams.set("t", `${Date.now()}`); | ||
| const response = await fetch(configUrl.toString()); | ||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
| return await response.json(); |
There was a problem hiding this comment.
loadAppConfig() fetches config.json from the built public root, but the repository’s dashboard/public/ directory doesn’t include a config.json file. This will make the feature silently fall back (and emit a warning) in default builds unless deployments are manually providing that file.
If runtime config via config.json is intended, consider adding a checked-in template (e.g. public/config.json with empty/default values) or documenting the required deployment step/location.
Description
This PR restores the dynamic configuration of the API Base URL in the dashboard, ensuring that developers can properly decouple the frontend from the backend and set custom
VITE_API_BASEvariables or configure it viaconfig.json.Changes
request.tsdynamic host resolution logic.main.tsnetwork interception.Summary by Sourcery
Restore configurable API base URL handling in the dashboard and align network requests and build tooling with the new configuration model.
New Features:
Enhancements: