codeant-ai-for-open-source[bot] commented on code in PR #37378:
URL: https://github.com/apache/superset/pull/37378#discussion_r2719689288
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -796,7 +803,17 @@ export class ThemeController {
this.loadFonts(fontUrls);
} catch (error) {
console.error('Failed to apply theme:', error);
- this.fallbackToDefaultMode();
+ // Re-throw the error so updateTheme can handle fallback logic
+ throw error;
+ }
+ }
+
+ private async applyThemeWithRecovery(theme: AnyThemeConfig): Promise<void> {
+ try {
+ const normalizedConfig = normalizeThemeConfig(theme);
+ this.globalTheme.setConfig(normalizedConfig);
+ } catch (error) {
+ await this.fallbackToDefaultMode();
Review Comment:
**Suggestion:** `applyThemeWithRecovery` catches errors and calls
`fallbackToDefaultMode`, but `fallbackToDefaultMode` itself calls
`applyThemeWithRecovery` for a fetched fresh theme — this creates possible
infinite recursion (fallback -> applyThemeWithRecovery -> fallback -> ...).
Instead propagate the error to the caller so the caller (which already has a
fallback try/catch) can handle it without recursive re-entry. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Theme recovery can recurse causing stack overflow.
- ❌ UI may become unresponsive during recovery attempts.
- ⚠️ Initial theme application and fallback paths affected.
- ⚠️ Error handling logic loops on invalid server themes.
```
</details>
```suggestion
// Propagate error to caller so caller's fallback logic can decide
next steps
throw error;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open superset-frontend/src/theme/ThemeController.ts and locate:
- fallbackToDefaultMode (starts around line 555) which, when it finds a
fresh system
theme, calls `await this.applyThemeWithRecovery(freshSystemTheme);`
(inside a
try/catch).
- applyThemeWithRecovery defined at lines 811-817.
2. Trigger a theme application failure so updateTheme's catch runs and calls
fallbackToDefaultMode (updateTheme catch at line ~546 -> calls
fallbackToDefaultMode).
3. fallbackToDefaultMode fetches a fresh system theme
(fetchSystemDefaultTheme at line
954). If the fetched theme is invalid in the same way (e.g., malformed
tokens),
applyThemeWithRecovery will catch and call fallbackToDefaultMode again.
4. This produces a recursive cycle: fallbackToDefaultMode ->
applyThemeWithRecovery ->
fallbackToDefaultMode -> ... until call stack exhaustion or synchronous
crash. The
recursion is concrete because fallbackToDefaultMode explicitly calls
applyThemeWithRecovery (lines ~561-566) and applyThemeWithRecovery currently
calls
fallbackToDefaultMode on error (lines 814-816).
5. Result: reproduceable stack overflow / unresponsive UI when a
server-provided system
theme is invalid or causes the same error as the active theme.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/theme/ThemeController.ts
**Line:** 816:816
**Comment:**
*Logic Error: `applyThemeWithRecovery` catches errors and calls
`fallbackToDefaultMode`, but `fallbackToDefaultMode` itself calls
`applyThemeWithRecovery` for a fetched fresh theme — this creates possible
infinite recursion (fallback -> applyThemeWithRecovery -> fallback -> ...).
Instead propagate the error to the caller so the caller (which already has a
fallback try/catch) can handle it without recursive re-entry.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
