codeant-ai-for-open-source[bot] commented on code in PR #38678:
URL: https://github.com/apache/superset/pull/38678#discussion_r2940308775
##########
Dockerfile:
##########
@@ -39,6 +39,11 @@ COPY docker/ /app/docker/
# Arguments for build configuration
ARG NPM_BUILD_CMD="build"
+# Switch to Tsinghua Debian mirror for faster apt downloads in China
+RUN sed -i 's/deb.debian.org/mirrors.tuna.tsinghua.edu.cn/g'
/etc/apt/sources.list.d/debian.sources 2>/dev/null \
+ || sed -i 's/deb.debian.org/mirrors.tuna.tsinghua.edu.cn/g'
/etc/apt/sources.list 2>/dev/null \
Review Comment:
**Suggestion:** This unconditionally rewrites Debian sources to a regional
mirror, so `apt-get update` can fail in environments where that mirror is
blocked or unavailable. Make the mirror host configurable and keep the default
as `deb.debian.org` so builds remain reliable globally. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ CI docker-build jobs fail during apt update.
- ❌ Local docker compose setup cannot build images.
- ⚠️ Node and Python stages both affected.
```
</details>
```suggestion
# Allow overriding Debian mirror for faster regional builds
ARG DEBIAN_MIRROR_HOST="deb.debian.org"
RUN sed -i "s|deb.debian.org|${DEBIAN_MIRROR_HOST}|g"
/etc/apt/sources.list.d/debian.sources 2>/dev/null \
|| sed -i "s|deb.debian.org|${DEBIAN_MIRROR_HOST}|g"
/etc/apt/sources.list 2>/dev/null \
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger a normal image build path used today, e.g. CI `docker buildx
build` in
`.github/workflows/superset-frontend.yml:48` (target `superset-node-ci` at
line 51) or
local `docker compose up --build` documented in
`docs/admin_docs/installation/docker-compose.mdx:82`.
2. Build executes `Dockerfile:42-45` (and again `Dockerfile:113-116`) which
unconditionally rewrites Debian host to `mirrors.tuna.tsinghua.edu.cn`.
3. Subsequent package installs call `/app/docker/apt-install.sh`
(`Dockerfile:48`,
`Dockerfile:204`, `Dockerfile:258`), and that script runs `apt-get update
-qq` at
`docker/apt-install.sh:39`.
4. In networks where Tsinghua mirror is blocked/unavailable, `apt-get
update` fails and
the Docker build stops, breaking both CI image builds and local compose
bootstrap.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** Dockerfile
**Line:** 42:44
**Comment:**
*Possible Bug: This unconditionally rewrites Debian sources to a
regional mirror, so `apt-get update` can fail in environments where that mirror
is blocked or unavailable. Make the mirror host configurable and keep the
default as `deb.debian.org` so builds remain reliable 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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=a93f9b6d28c8f0cfd8b8d5989e677386b42e8ca87fdf6f1848c71df9ab421662&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=a93f9b6d28c8f0cfd8b8d5989e677386b42e8ca87fdf6f1848c71df9ab421662&reaction=dislike'>👎</a>
##########
superset-frontend/src/hooks/apiResources/tables.ts:
##########
@@ -115,20 +120,24 @@ const tableApi = api.injectEndpoints({
hasMore: json.count > json.result.length,
}),
}),
- serializeQueryArgs: ({ queryArgs: { dbId, schema } }) => ({
+ serializeQueryArgs: ({
+ queryArgs: { dbId, schema },
+ }: {
+ queryArgs: Pick<FetchTablesQueryParams, 'dbId' | 'schema'>;
+ }) => ({
dbId,
schema,
}),
Review Comment:
**Suggestion:** The cache key for table queries ignores the selected
catalog, so switching catalogs while keeping the same database and schema can
reuse stale cached table results from the previous catalog instead of fetching
fresh data. Include `catalog` in `serializeQueryArgs` so cache entries are
isolated per catalog. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Add Dataset table list can show wrong catalog tables.
- ❌ Datasource Editor may bind users to incorrect table.
- ⚠️ Users must manual refresh to recover correct list.
```
</details>
```suggestion
serializeQueryArgs: ({
queryArgs: { dbId, catalog, schema },
}: {
queryArgs: Pick<FetchTablesQueryParams, 'dbId' | 'catalog' |
'schema'>;
}) => ({
dbId,
catalog,
schema,
}),
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a real TableSelector entry flow such as Add Dataset left panel
(`superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx:206`)
or
Datasource Editor
(`superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:2062`),
both of which render `<TableSelector>`.
2. In `TableSelector`, table loading uses `useTables({ dbId, catalog:
currentCatalog,
schema: currentSchema })`
(`superset-frontend/src/components/TableSelector/index.tsx:189-192`), which
calls
`useTablesQuery` in `tables.ts`
(`superset-frontend/src/hooks/apiResources/tables.ts:197-199`).
3. The request includes catalog in URL params (`catalog_name`)
(`tables.ts:116`), but
cache key serialization only uses `{ dbId, schema }` (`tables.ts:123-130`),
so different
catalogs with same db+schema share one cache entry.
4. Reproduce by selecting catalog A + schema S, then switching catalog and
re-selecting
schema S (catalog reset behavior is in `TableSelector`
`internalCatalogChange`,
`index.tsx:274-281`): `useTablesQuery` can serve cached results from catalog
A instead of
fetching catalog B tables, showing stale table options.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/hooks/apiResources/tables.ts
**Line:** 123:130
**Comment:**
*Logic Error: The cache key for table queries ignores the selected
catalog, so switching catalogs while keeping the same database and schema can
reuse stale cached table results from the previous catalog instead of fetching
fresh data. Include `catalog` in `serializeQueryArgs` so cache entries are
isolated per catalog.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=d3ad287008b55659dd88fe1b022dbe63d5f668aacc52597da542e7a64cd0cd1e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=d3ad287008b55659dd88fe1b022dbe63d5f668aacc52597da542e7a64cd0cd1e&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useAnnotations.ts:
##########
@@ -52,7 +52,17 @@ export function useAnnotations(params:
FetchValidationQueryParams) {
},
{
skip: !(hasValidator && dbId && sql),
- selectFromResult: ({ isLoading, isError, error, data }) => {
+ selectFromResult: ({
Review Comment:
**Suggestion:** The selector builds a new array with `data.map(...)` on
every selector run, which breaks RTK Query's shallow memoization for
`selectFromResult` and can cause unnecessary re-renders on unrelated Redux
updates. Cache the mapped annotations by `data` reference inside a selector
factory so the returned `data` reference stays stable when the source data has
not changed. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab editor does extra annotation recomputation work.
- ⚠️ Frequent selection updates can trigger unnecessary editor updates.
```
</details>
##########
superset-frontend/src/hooks/apiResources/queries.ts:
##########
@@ -162,12 +166,22 @@ const queryHistoryApi = api.injectEndpoints({
result: json.result.map(mapQueryResponse),
}),
}),
- serializeQueryArgs: ({ queryArgs: { editorId } }) => ({ editorId }),
+ serializeQueryArgs: ({
+ queryArgs: { editorId },
+ }: {
+ queryArgs: Pick<EditorQueriesParams, 'editorId'>;
+ }) => ({ editorId }),
// Refetch when the page arg changes
- forceRefetch({ currentArg, previousArg }) {
+ forceRefetch({
+ currentArg,
+ previousArg,
+ }: {
+ currentArg?: EditorQueriesParams;
+ previousArg?: EditorQueriesParams;
+ }) {
return currentArg !== previousArg;
},
- merge: (currentCache, newItems) => {
+ merge: (currentCache: QueryResult, newItems: QueryResult) => {
currentCache.result.push(...newItems.result);
},
Review Comment:
**Suggestion:** The pagination merge only appends `result` and never
refreshes `count`, but the UI uses `count` to decide whether to keep loading
more pages. If total rows change between requests (common in query history),
this stale value can cause premature stop or repeated extra fetches. Update
`count` from each new page before appending results. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab history can stop before all rows.
- ⚠️ Infinite scroll can trigger unnecessary extra requests.
- ❌ Query history list may be incomplete for users.
```
</details>
```suggestion
merge: (currentCache: QueryResult, newItems: QueryResult) => {
currentCache.count = newItems.count;
currentCache.result.push(...newItems.result);
},
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab Query History with backend persistence enabled;
`QueryHistory` calls
`useEditorQueriesQuery({ editorId, pageIndex })` at
`superset-frontend/src/SqlLab/components/QueryHistory/index.tsx:80-85`.
2. Scroll to bottom so pagination runs; the effect at `index.tsx:113-117`
loads next pages
while `loadedDataCount < totalCount`, where `totalCount` is `data?.count`
from
`index.tsx:111`.
3. The RTK query cache merge at
`superset-frontend/src/hooks/apiResources/queries.ts:184-186` appends
`result` but never
updates `count`, so cache keeps first page's total.
4. If later page responses return a different total (realistic in query
history as rows
are added/removed while viewing), pagination decisions use stale
`totalCount`, causing
either premature stop (missing rows) or extra repeated fetches.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/hooks/apiResources/queries.ts
**Line:** 184:186
**Comment:**
*Logic Error: The pagination merge only appends `result` and never
refreshes `count`, but the UI uses `count` to decide whether to keep loading
more pages. If total rows change between requests (common in query history),
this stale value can cause premature stop or repeated extra fetches. Update
`count` from each new page before appending results.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=bef23bddbd7cee1de7d1b6d529d6b88f3fe467cc3b293106d7ed7d2a40f515d3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=bef23bddbd7cee1de7d1b6d529d6b88f3fe467cc3b293106d7ed7d2a40f515d3&reaction=dislike'>👎</a>
##########
Dockerfile:
##########
@@ -113,7 +123,10 @@ RUN useradd --user-group -d ${SUPERSET_HOME} -m
--no-log-init --shell /bin/bash
# Some bash scripts needed throughout the layers
COPY --chmod=755 docker/*.sh /app/docker/
-RUN pip install --no-cache-dir --upgrade uv
+RUN pip install --no-cache-dir --upgrade uv -i
https://pypi.tuna.tsinghua.edu.cn/simple
+
+# Configure uv to use Tsinghua mirror for all subsequent installs
+ENV UV_INDEX_URL=https://pypi.tuna.tsinghua.edu.cn/simple
Review Comment:
**Suggestion:** Python package installation is pinned to a single regional
index for both `pip` and `uv`, so dependency installation fails when that index
is inaccessible. Use a configurable index URL with a global default to avoid
build outages outside that region. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Python dependency installation fails across build stages.
- ❌ GitHub docker workflow cannot publish built images.
- ⚠️ Developer docker compose bootstrap blocked by index reachability.
```
</details>
```suggestion
ARG PYPI_INDEX_URL="https://pypi.org/simple";
RUN pip install --no-cache-dir --upgrade uv -i ${PYPI_INDEX_URL}
# Configure uv to use the configured package index
ENV UV_INDEX_URL=${PYPI_INDEX_URL}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start a standard Docker build flow used by project automation:
`.github/workflows/docker.yml:67-89` builds images, and developers run
`docker compose up
--build` per `docs/admin_docs/installation/docker-compose.mdx:82`.
2. During `python-base` stage, `Dockerfile:126` installs `uv` via a
hardcoded Tsinghua
PyPI URL, then `Dockerfile:129` sets global `UV_INDEX_URL` to the same
mirror.
3. Later dependency steps rely on `uv` index configuration
(`Dockerfile:187`, `247`,
`275`, `277`, `287`, `296` all run `uv pip install ...`).
4. If `https://pypi.tuna.tsinghua.edu.cn/simple` is inaccessible, `pip
install uv` or any
later `uv pip install` fails, aborting image creation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** Dockerfile
**Line:** 126:129
**Comment:**
*Possible Bug: Python package installation is pinned to a single
regional index for both `pip` and `uv`, so dependency installation fails when
that index is inaccessible. Use a configurable index URL with a global default
to avoid build outages outside that region.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=593a05a7672002907cfe44e2847d5d8f18ff0faee852258a3465f8dec897889e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=593a05a7672002907cfe44e2847d5d8f18ff0faee852258a3465f8dec897889e&reaction=dislike'>👎</a>
##########
superset-frontend/src/hooks/apiResources/catalogs.ts:
##########
@@ -87,14 +91,24 @@ export function useCatalogs(options: Params) {
const fetchData = useEffectEvent(
(dbId: FetchCatalogsQueryParams['dbId'], forceRefresh = false) => {
if (dbId && (!result.currentData || forceRefresh)) {
- trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) =>
{
- if (isSuccess) {
- onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
- }
- if (isError) {
- onError?.(result.error as ClientErrorObject);
- }
- });
+ trigger({ dbId, forceRefresh }).then(
+ ({
+ isSuccess,
+ isError,
+ data,
+ }: {
+ isSuccess: boolean;
+ isError: boolean;
+ data?: CatalogOption[];
+ }) => {
+ if (isSuccess) {
+ onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
+ }
+ if (isError) {
+ onError?.(result.error as ClientErrorObject);
+ }
Review Comment:
**Suggestion:** The lazy query error handler reads `result.error` from the
hook state instead of the error returned by the `trigger` promise. During a
failed refetch, `result.error` can still be stale or undefined, so the callback
may receive the wrong error payload and show an incorrect message. Use the
`error` field from the resolved trigger result. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Catalog refresh may show wrong error details.
- ⚠️ SQL Lab selector error messaging becomes unreliable.
- ⚠️ Datasource/Table selector diagnostics become less actionable.
```
</details>
```suggestion
({
isSuccess,
isError,
data,
error,
}: {
isSuccess: boolean;
isError: boolean;
data?: CatalogOption[];
error?: ClientErrorObject;
}) => {
if (isSuccess) {
onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
}
if (isError) {
onError?.(error as ClientErrorObject);
}
}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a screen using `DatabaseSelector` (used in
`src/SqlLab/components/SqlEditorLeftBar/index.tsx:137`, `:190`,
`src/components/TableSelector/index.tsx:353`, and
`src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:1865`).
2. In `src/components/DatabaseSelector/index.tsx:487`, click refresh
(`onClick={refetchCatalogs}`), which calls `useCatalogs().refetch` from
`src/hooks/apiResources/catalogs.ts:116-118`.
3. `refetch` executes `fetchData(dbId, true)` and then `trigger({ dbId,
forceRefresh
}).then(...)` at `catalogs.ts:94`; when the request fails, callback enters
`isError`
branch at `catalogs.ts:107`.
4. The error callback currently passes `result.error` (`catalogs.ts:108`)
instead of the
lazy trigger result error. This can be stale/undefined for that request, so
`DatabaseSelector` onError (`index.tsx:80-87`) may show generic/wrong error
handling.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/hooks/apiResources/catalogs.ts
**Line:** 95:109
**Comment:**
*Logic Error: The lazy query error handler reads `result.error` from
the hook state instead of the error returned by the `trigger` promise. During a
failed refetch, `result.error` can still be stale or undefined, so the callback
may receive the wrong error payload and show an incorrect message. Use the
`error` field from the resolved trigger result.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=172ec8b8c5ac64f08ea447c73a0415136206950da5e82a5806d3237a87f9fa5e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=172ec8b8c5ac64f08ea447c73a0415136206950da5e82a5806d3237a87f9fa5e&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-cartodiagram/src/components/ChartWrapper.tsx:
##########
@@ -45,9 +45,9 @@ export const ChartWrapper: FC<ChartWrapperProps> = ({
// Create a mock store that is needed by
// eCharts components to access the locale.
- const mockStore = configureStore({
- reducer: (state = { common: { locale } }) => state,
- });
+ const mockStore = createStore(
+ (state = { common: { locale } }) => state,
+ );
Review Comment:
**Suggestion:** Creating the Redux store during every render causes a
brand-new store instance each time `ChartWrapper` re-renders, which forces
connected children to resubscribe/reset and can lead to avoidable render churn.
Initialize the store once with a lazy state initializer so the store identity
stays stable across renders. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Cartodiagram charts recreate Redux store every rerender.
- ⚠️ Embedded chart subtree resubscribes through Provider repeatedly.
- ⚠️ Map interactions may feel slower with many overlays.
```
</details>
```suggestion
const [mockStore] = useState(() =>
createStore((state = { common: { locale } }) => state),
);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a Cartodiagram chart in Superset Explore/Dashboard; this viz is
registered in
`superset-frontend/src/visualizations/presets/MainPreset.ts:199-211` and
loaded via
`loadChart` in
`superset-frontend/plugins/plugin-chart-cartodiagram/src/plugin/index.ts:71-77`.
2. Rendering reaches `CartodiagramPlugin`
(`.../src/CartodiagramPlugin.tsx:55-57`) which
mounts `OlChartMap`; `OlChartMap` then creates `ChartLayer`
(`.../src/components/OlChartMap.tsx:37-50` in second effect chunk).
3. `ChartLayer` renders embedded charts with `ReactDOM.render` in
`.../src/components/ChartLayer.tsx:186-194` (and update path `222-230`),
which calls
`createChartComponent` in `.../src/util/chartUtil.tsx:41-48`, returning
`ChartWrapper`.
4. `ChartWrapper` first renders, then re-renders after `setChart`
(`.../src/components/ChartWrapper.tsx:39, 42-44`); on each render it executes
`createStore` at `:48-50`, creating a new Redux store identity and forcing
provider
subtree resubscription churn.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-cartodiagram/src/components/ChartWrapper.tsx
**Line:** 48:50
**Comment:**
*Logic Error: Creating the Redux store during every render causes a
brand-new store instance each time `ChartWrapper` re-renders, which forces
connected children to resubscribe/reset and can lead to avoidable render churn.
Initialize the store once with a lazy state initializer so the store identity
stays stable across renders.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=2d3eb87d17a59e23f0b424d5bac97ae9d8d9a393598c8c119974c860ee405d68&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38678&comment_hash=2d3eb87d17a59e23f0b424d5bac97ae9d8d9a393598c8c119974c860ee405d68&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/TableExploreTree/useTreeData.ts:
##########
@@ -160,12 +162,12 @@ const useTreeData = ({
},
true,
)
- .then(({ data }) => {
+ .then(({ data }: { data?: TableOptionsData }) => {
Review Comment:
**Suggestion:** The lazy RTK query trigger resolves with `{ isSuccess,
isError, data, error }` even on failures, so relying on `.catch(...)` means API
errors are silently ignored and `errorPayload` is never set. Handle `isError`
inside `.then(...)` to surface failures correctly. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab schema-load failures show no error banner.
- ⚠️ Users retry blindly when tables API fails.
```
</details>
##########
superset-frontend/src/hooks/apiResources/schemas.ts:
##########
@@ -96,7 +100,15 @@ export function useSchemas(options: Params) {
) => {
if (dbId && (!result.currentData || forceRefresh)) {
trigger({ dbId, catalog, forceRefresh }).then(
- ({ isSuccess, isError, data }) => {
+ ({
Review Comment:
**Suggestion:** The lazy query error handler is wired to `result.error`
(state from `useSchemasQuery`) instead of the error returned by the current
`trigger(...)` call. When a refetch fails, this can pass a stale/undefined
error object to `onError`, causing incorrect error reporting. Capture `error`
from the trigger result and pass that to `onError`. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Schema refresh may lose backend error details.
- ⚠️ DatabaseSelector may show generic error fallback message.
```
</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]
