codeant-ai-for-open-source[bot] commented on code in PR #40443:
URL: https://github.com/apache/superset/pull/40443#discussion_r3304482336
##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -53,11 +64,40 @@ const ExtensionsStartup: React.FC<{ children?:
React.ReactNode }> = ({
children,
}) => {
const [initialized, setInitialized] = useState(false);
+ const location = useLocation();
+ const prevPathname = useRef<string | null>(null);
const userId = useSelector<RootState, number | undefined>(
({ user }) => user.userId,
);
+ // Notify the navigation namespace on every route change.
+ useEffect(() => {
+ if (prevPathname.current !== location.pathname) {
+ prevPathname.current = location.pathname;
+ notifyPageChange(location.pathname);
+ }
+ }, [location.pathname]);
+
+ // Isolate unhandled rejections from extension code for the lifetime of the
+ // app — registered once, never removed.
+ useEffect(() => {
+ const handleUnhandledRejection = (event: PromiseRejectionEvent) => {
+ logging.error(
+ '[extensions] Unhandled rejection from extension:',
+ event.reason,
+ );
+ event.preventDefault();
+ };
Review Comment:
**Suggestion:** The global `unhandledrejection` handler calls
`preventDefault()` for every unhandled promise rejection in the app, not just
extension-originated failures. This suppresses the browser's default error
surfacing and can hide real production failures from existing global
reporting/debugging flows. Limit suppression to verified extension errors (or
just log without preventing default globally). [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Browser devtools may hide unhandled rejection error reporting.
- ⚠️ Any external monitoring relying on default handler may miss failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load the Superset frontend so `App` renders at
`superset-frontend/src/views/App.tsx:43-88`, which wraps all routes and
`ChatbotMount`
inside `<ExtensionsStartup>` at `App.tsx:52-84`.
2. When `ExtensionsStartup` mounts, its `useEffect` at
`superset-frontend/src/extensions/ExtensionsStartup.tsx:82-99` registers
`window.addEventListener('unhandledrejection', handleUnhandledRejection)`
with the handler
defined at lines 85-91.
3. On any page (any route component under the `<Switch>` at
`App.tsx:53-76`), trigger an
unhandled promise rejection in the browser context (for example by running
`Promise.reject(new Error('test'))` in the devtools console without a
`.catch()`), which
causes the browser to fire a global `unhandledrejection` event on `window`.
4. The registered handler in `ExtensionsStartup.tsx:85-91` logs via
`logging.error('[extensions] Unhandled rejection from extension:',
event.reason)` and then
calls `event.preventDefault()`, which marks the rejection as handled and
suppresses the
browser's default unhandled-rejection reporting for *all* such errors,
including those
originating from core app code and non-extension logic.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2b45dac89fd24c5ab50ec997b1118370&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2b45dac89fd24c5ab50ec997b1118370&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/extensions/ExtensionsStartup.tsx
**Line:** 85:91
**Comment:**
*Logic Error: The global `unhandledrejection` handler calls
`preventDefault()` for every unhandled promise rejection in the app, not just
extension-originated failures. This suppresses the browser's default error
surfacing and can hide real production failures from existing global
reporting/debugging flows. Limit suppression to verified extension errors (or
just log without preventing default globally).
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=575e823ed7470913c2ecf0f3211e548d6b52128d5f807d6c74d2b7f43ffd26fe&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=575e823ed7470913c2ecf0f3211e548d6b52128d5f807d6c74d2b7f43ffd26fe&reaction=dislike'>👎</a>
##########
superset/extensions/settings.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Admin settings persistence for extensions (active chatbot,
enable/disable)."""
+
+from typing import Any
+
+from sqlalchemy.dialects.postgresql import insert as pg_insert
+from sqlalchemy.dialects.sqlite import insert as sqlite_insert
+
+from superset import db
+from superset.models.core import ExtensionEnabled, ExtensionSettings
+from superset.utils.decorators import transaction
+
+_SETTINGS_ROW_ID = 1
+
+
+def get_extension_settings() -> dict[str, Any]:
+ row = db.session.get(ExtensionSettings, _SETTINGS_ROW_ID)
+ enabled_rows = db.session.query(ExtensionEnabled).all()
+ return {
+ "active_chatbot_id": row.active_chatbot_id if row else None,
+ "enabled": {r.extension_id: r.enabled for r in enabled_rows},
+ }
+
+
+def _upsert_settings_row(
+ active_chatbot_id: str | None,
+) -> None:
+ """Upsert the singleton settings row without a read-then-insert race."""
+ bind = db.session.get_bind()
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ stmt = (
+ pg_insert(ExtensionSettings)
+ .values(id=_SETTINGS_ROW_ID, active_chatbot_id=active_chatbot_id)
+ .on_conflict_do_update(
+ index_elements=["id"],
+ set_={"active_chatbot_id": active_chatbot_id},
+ )
+ )
+ db.session.execute(stmt)
+ else:
+ stmt = (
+ sqlite_insert(ExtensionSettings)
+ .values(id=_SETTINGS_ROW_ID, active_chatbot_id=active_chatbot_id)
+ .on_conflict_do_update(
+ index_elements=["id"],
+ set_={"active_chatbot_id": active_chatbot_id},
+ )
+ )
+ db.session.execute(stmt)
Review Comment:
**Suggestion:** The fallback branch treats every non-PostgreSQL engine as
SQLite and builds a SQLite-specific `INSERT ... ON CONFLICT` statement. On
deployments using MySQL/MariaDB (supported Superset metadata backends), this
statement compilation/execution will fail at runtime, so updating
`active_chatbot_id` can break with a database error. Add explicit dialect
handling (or a generic ORM fallback) instead of routing all non-Postgres
engines to `sqlite_insert`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Admin extension settings update fails on MySQL metadata databases.
- ⚠️ Active chatbot selection cannot be persisted for administrators.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset to use a MySQL or MariaDB metadata database (via
`SQLALCHEMY_DATABASE_URI`) and run the migration that creates
`ExtensionSettings` (see
`superset/migrations/versions/2026-05-25_00-00_b2c3d4e5f6a7_add_extension_settings.py`).
2. Start the backend and, as an admin user, call the HTTP endpoint `PUT
/api/v1/extensions/settings` (wired to `ExtensionsRestApi.put_settings` at
`superset/extensions/api.py:189-220`) with a JSON body containing
`"active_chatbot_id":
"my-bot"` so that the request passes the admin check at `api.py:216-217` and
reaches
`update_extension_settings(body)` at `api.py:219`.
3. Inside `update_extension_settings`
(`superset/extensions/settings.py:95-108`), the
presence of `"active_chatbot_id"` in the body causes
`_upsert_settings_row(active_chatbot_id)` to be called at lines 97-100,
which then
inspects the SQLAlchemy engine's dialect in `_upsert_settings_row` at lines
45-47.
4. Because the engine dialect name is `"mysql"` (not `"postgresql"`),
execution follows
the `else:` branch at `settings.py:57-66`, constructing a statement with
`sqlite_insert(ExtensionSettings).on_conflict_do_update(...)` that uses
SQLite-specific
upsert semantics; when SQLAlchemy attempts to compile/execute this against
the MySQL
engine, it will raise a compilation or execution error, causing the
`/api/v1/extensions/settings` PUT request to fail (typically with HTTP 500)
and preventing
`active_chatbot_id` from being persisted.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=73275874616c4190a3bcfbfca3b32d2f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=73275874616c4190a3bcfbfca3b32d2f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/settings.py
**Line:** 57:66
**Comment:**
*Api Mismatch: The fallback branch treats every non-PostgreSQL engine
as SQLite and builds a SQLite-specific `INSERT ... ON CONFLICT` statement. On
deployments using MySQL/MariaDB (supported Superset metadata backends), this
statement compilation/execution will fail at runtime, so updating
`active_chatbot_id` can break with a database error. Add explicit dialect
handling (or a generic ORM fallback) instead of routing all non-Postgres
engines to `sqlite_insert`.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=cd299da9c8c1e288bd72737c6b9524fffc8edae531487db999170eacf3e77366&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=cd299da9c8c1e288bd72737c6b9524fffc8edae531487db999170eacf3e77366&reaction=dislike'>👎</a>
##########
superset/extensions/settings.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Admin settings persistence for extensions (active chatbot,
enable/disable)."""
+
+from typing import Any
+
+from sqlalchemy.dialects.postgresql import insert as pg_insert
+from sqlalchemy.dialects.sqlite import insert as sqlite_insert
+
+from superset import db
+from superset.models.core import ExtensionEnabled, ExtensionSettings
+from superset.utils.decorators import transaction
+
+_SETTINGS_ROW_ID = 1
+
+
+def get_extension_settings() -> dict[str, Any]:
+ row = db.session.get(ExtensionSettings, _SETTINGS_ROW_ID)
+ enabled_rows = db.session.query(ExtensionEnabled).all()
+ return {
+ "active_chatbot_id": row.active_chatbot_id if row else None,
+ "enabled": {r.extension_id: r.enabled for r in enabled_rows},
+ }
+
+
+def _upsert_settings_row(
+ active_chatbot_id: str | None,
+) -> None:
+ """Upsert the singleton settings row without a read-then-insert race."""
+ bind = db.session.get_bind()
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ stmt = (
+ pg_insert(ExtensionSettings)
+ .values(id=_SETTINGS_ROW_ID, active_chatbot_id=active_chatbot_id)
+ .on_conflict_do_update(
+ index_elements=["id"],
+ set_={"active_chatbot_id": active_chatbot_id},
+ )
+ )
+ db.session.execute(stmt)
+ else:
+ stmt = (
+ sqlite_insert(ExtensionSettings)
+ .values(id=_SETTINGS_ROW_ID, active_chatbot_id=active_chatbot_id)
+ .on_conflict_do_update(
+ index_elements=["id"],
+ set_={"active_chatbot_id": active_chatbot_id},
+ )
+ )
+ db.session.execute(stmt)
+
+
+def _upsert_enabled_flag(extension_id: str, enabled: bool) -> None:
+ """Upsert a per-extension enabled flag without a read-then-insert race."""
+ bind = db.session.get_bind()
+ dialect = bind.dialect.name
+ if dialect == "postgresql":
+ stmt = (
+ pg_insert(ExtensionEnabled)
+ .values(extension_id=extension_id, enabled=enabled)
+ .on_conflict_do_update(
+ index_elements=["extension_id"],
+ set_={"enabled": enabled},
+ )
+ )
+ db.session.execute(stmt)
+ else:
+ stmt = (
+ sqlite_insert(ExtensionEnabled)
+ .values(extension_id=extension_id, enabled=enabled)
+ .on_conflict_do_update(
+ index_elements=["extension_id"],
+ set_={"enabled": enabled},
+ )
+ )
+ db.session.execute(stmt)
Review Comment:
**Suggestion:** This upsert path has the same dialect bug for per-extension
flags: any non-PostgreSQL engine is forced through SQLite SQL generation. On
MySQL/MariaDB metadata DBs, writes to the `enabled` map can fail at runtime
because SQLite conflict syntax is not valid there. Add proper per-dialect
handling rather than defaulting to `sqlite_insert`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Per-extension enabled flags fail to persist on MySQL metadata.
- ⚠️ Admin UI toggles appear ignored, confusing administrators.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run Superset against a MySQL or MariaDB metadata database (configured via
`SQLALCHEMY_DATABASE_URI`) with the `ExtensionEnabled` table created by the
same migration
referenced above.
2. As an admin user, send `PUT /api/v1/extensions/settings` to
`ExtensionsRestApi.put_settings` in `superset/extensions/api.py:189-220`
with a body that
includes an `"enabled"` map such as `{ "enabled": { "publisher.myext": true
} }`, so the
request passes the admin check at `api.py:216-217` and calls
`update_extension_settings(body)` at `api.py:219`.
3. In `update_extension_settings`
(`superset/extensions/settings.py:95-108`), the
`"enabled"` key being a dict causes the loop at lines 102-106 to iterate
over each
`(extension_id, enabled)` pair and call `_upsert_enabled_flag(extension_id,
enabled)` for
those where `enabled` is a boolean.
4. Inside `_upsert_enabled_flag` (`settings.py:69-92`), the engine dialect
name `"mysql"`
again routes execution into the `else:` block at lines 83-92, which builds a
statement
using `sqlite_insert(ExtensionEnabled).on_conflict_do_update(...)`;
SQLAlchemy then
attempts to execute this SQLite-specific upsert against a MySQL engine,
leading to a
runtime error and causing the `/api/v1/extensions/settings` PUT call to fail
when writing
per-extension enabled flags, so admin toggle changes are not saved.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9373c47e36c242a6a5cfb360e4364dd7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9373c47e36c242a6a5cfb360e4364dd7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/settings.py
**Line:** 83:92
**Comment:**
*Api Mismatch: This upsert path has the same dialect bug for
per-extension flags: any non-PostgreSQL engine is forced through SQLite SQL
generation. On MySQL/MariaDB metadata DBs, writes to the `enabled` map can fail
at runtime because SQLite conflict syntax is not valid there. Add proper
per-dialect handling rather than defaulting to `sqlite_insert`.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=3c23d40bc5cae00c11a3e50eed666984062becdf1dfc074056208dd6c51609c9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40443&comment_hash=3c23d40bc5cae00c11a3e50eed666984062becdf1dfc074056208dd6c51609c9&reaction=dislike'>👎</a>
--
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]
