bito-code-review[bot] commented on code in PR #37378:
URL: https://github.com/apache/superset/pull/37378#discussion_r2756251722
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -143,8 +143,26 @@ export class ThemeController {
// Setup change callback
if (onChange) this.onChangeCallbacks.add(onChange);
- // Apply initial theme and persist mode
- this.applyTheme(initialTheme);
+ // Apply initial theme with recovery for corrupted stored themes
+ try {
+ this.applyTheme(initialTheme);
+ } catch (error) {
+ // Corrupted dev override or CRUD theme in storage - clear and retry
with defaults
+ console.warn(
+ 'Failed to apply stored theme, clearing invalid overrides:',
+ error,
+ );
+ this.devThemeOverride = null;
+ this.crudThemeId = null;
+ this.storage.removeItem(STORAGE_KEYS.DEV_THEME_OVERRIDE);
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.storage.removeItem(STORAGE_KEYS.APPLIED_THEME_ID);
+
+ // Retry with clean default theme
+ this.currentMode = ThemeMode.DEFAULT;
+ const safeTheme = this.defaultTheme || {};
+ this.applyTheme(safeTheme);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unhandled error in constructor</b></div>
<div id="fix">
If the retry theme application fails in the constructor, an unhandled error
is thrown, potentially breaking initialization.
</div>
</div>
<small><i>Code Review Run #ad4e65</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -896,25 +936,82 @@ export class ThemeController {
/**
* Fetches a theme configuration from the CRUD API.
* @param themeId - The ID of the theme to fetch
- * @returns The theme configuration or null if not found
+ * @returns The theme configuration or null if fetch fails
*/
private async fetchCrudTheme(
themeId: string,
): Promise<AnyThemeConfig | null> {
try {
// Use SupersetClient for proper authentication handling
- const getTheme = makeApi<void, { result: { json_data: string } }>({
+ const getTheme = makeApi<
+ void,
+ { result: { json_data: string; theme_name?: string } }
+ >({
method: 'GET',
endpoint: `/api/v1/theme/${themeId}`,
});
const { result } = await getTheme();
const themeConfig = JSON.parse(result.json_data);
+ if (!themeConfig || typeof themeConfig !== 'object') {
+ console.error(`Invalid theme configuration for theme ${themeId}`);
+ return null;
+ }
+
+ // Return theme as-is
+ // Invalid tokens will be handled by Ant Design at runtime
+ // Runtime errors will be caught by applyThemeWithRecovery()
return themeConfig;
} catch (error) {
console.error('Failed to fetch CRUD theme:', error);
return null;
}
}
+
+ /**
+ * Fetches a fresh system default theme from the API for runtime recovery.
+ * Tries multiple fallback strategies to find a valid theme.
+ *
+ * Note: Uses raw fetch() instead of SupersetClient because ThemeController
+ * initializes early in the app lifecycle, before SupersetClient is fully
+ * configured. This avoids boot-time circular dependencies.
+ *
+ * @returns The system default theme configuration or null if not found
+ */
+ private async fetchSystemDefaultTheme(): Promise<AnyThemeConfig | null> {
+ try {
+ // Try to fetch theme marked as system default (is_system_default=true)
+ const defaultResponse = await fetch(
+
'/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t)))',
+ );
+ if (defaultResponse.ok) {
+ const data = await defaultResponse.json();
+ if (data.result?.length > 0) {
+ const themeConfig = JSON.parse(data.result[0].json_data);
+ if (themeConfig && typeof themeConfig === 'object') {
+ return themeConfig;
+ }
+ }
+ }
+
+ // Fallback: Try to fetch system theme named 'THEME_DEFAULT'
+ const fallbackResponse = await fetch(
+
'/api/v1/theme/?q=(filters:!((col:theme_name,opr:eq,value:THEME_DEFAULT),(col:is_system,opr:eq,value:!t)))',
+ );
+ if (fallbackResponse.ok) {
+ const fallbackData = await fallbackResponse.json();
+ if (fallbackData.result?.length > 0) {
+ const themeConfig = JSON.parse(fallbackData.result[0].json_data);
+ if (themeConfig && typeof themeConfig === 'object') {
+ return themeConfig;
+ }
+ }
+ }
+ } catch (error) {
+ // Silently handle fetch errors
+ }
+
+ return null;
+ }
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent auth in API calls</b></div>
<div id="fix">
fetchSystemDefaultTheme uses unauthenticated fetch while other theme API
calls use makeApi, potentially causing auth failures for protected endpoints.
</div>
</div>
<small><i>Code Review Run #ad4e65</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
