codeant-ai-for-open-source[bot] commented on code in PR #42033:
URL: https://github.com/apache/superset/pull/42033#discussion_r3577555759
##########
superset-frontend/src/middleware/asyncEvent.ts:
##########
@@ -94,32 +96,71 @@ const fetchCachedData = async (
return { status, data };
};
-export const waitForAsyncData = async (asyncResponse: AsyncEvent) =>
+export const waitForAsyncData = async (
+ asyncResponse: AsyncEvent,
+ signal?: AbortSignal,
+) =>
new Promise((resolve, reject) => {
const jobId = asyncResponse.job_id;
+
+ let onAbort: (() => void) | undefined;
+ const cleanup = () => {
+ removeListener(jobId);
+ if (onAbort && signal) {
+ signal.removeEventListener('abort', onAbort);
+ }
+ };
+
+ // Bail immediately if the caller has already aborted (e.g. the chart was
+ // unmounted before the job started), avoiding a leaked listener.
+ if (signal?.aborted) {
+ reject(new DOMException('Aborted', 'AbortError'));
+ return;
+ }
+
const listener = async (asyncEvent: AsyncEvent) => {
switch (asyncEvent.status) {
case JOB_STATUS.DONE: {
- let { data, status } = await fetchCachedData(asyncEvent); //
eslint-disable-line prefer-const
+ // Forward the signal so the cached-result download is cancelled too
if
+ // the caller aborts mid-fetch, rather than wasting
network/processing.
+ let { data, status } = await fetchCachedData(asyncEvent, signal); //
eslint-disable-line prefer-const
data = ensureIsArray(data);
if (status === 'success') {
resolve(data);
} else {
reject(data);
}
+ // Terminal status: the promise is settled, so fully clean up.
+ cleanup();
break;
}
case JOB_STATUS.ERROR: {
const err = parseErrorJson(asyncEvent);
reject(err);
+ // Terminal status: the promise is settled, so fully clean up.
+ cleanup();
break;
}
default: {
logging.warn('received event with status', asyncEvent.status);
+ // Non-terminal status: drop this (stale) job listener but keep the
+ // abort handler so an unmount/supersede can still reject the pending
+ // promise (full cleanup here would leave it hanging on abort).
+ removeListener(jobId);
Review Comment:
**Suggestion:** In the non-terminal status branch, the listener is removed
immediately, so if a job emits `pending`/`running` before `done`, the
subsequent terminal event has no listener and the waiting promise can hang
forever. Keep the listener registered for non-terminal states and only clean it
up on terminal states (`done`, `error`) or abort. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Async chart queries can hang waiting for results.
❌ Native filter values never resolve under async queries.
⚠️ Async event retries log noisy 'listener not found' warnings.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable GLOBAL_ASYNC_QUERIES so async events middleware is active, as
wired by `init()`
in `superset-frontend/src/middleware/asyncEvent.ts` (lines ~12-36) and the
backend
`AsyncEventsRestApi.events` endpoint in `superset/async_events/api.py:35-41`.
2. Trigger a chart data request that returns HTTP 202 with async job
metadata, which flows
into `handleChartDataResponse` in
`superset-frontend/src/components/Chart/chartAction.ts:17-45`; for status
202 it calls
`waitForAsyncData(result, signal)` to await async results.
3. For that job_id, let the backend emit a non-terminal async event (e.g.
status "pending"
or "running") via `/api/v1/async_event/` so the frontend receives it and
passes it to
`processEvents` in `superset-frontend/src/middleware/asyncEvent.ts:184-207`,
which looks
up `listenersByJobId[jobId]` and invokes the listener created inside
`waitForAsyncData`.
4. When the listener runs, `switch (asyncEvent.status)` in
`asyncEvent.ts:12-41` hits the
`default` branch for the non-terminal status, logs the warning and calls
`removeListener(jobId)` (lines 145-149 in the PR hunk) without resolving or
rejecting the
promise; subsequent terminal events (`status: "done"` or `"error"`) for the
same job_id
are processed by `processEvents` but find no listener, fall into the
retry/race-condition
branch, and ultimately log "listener not found for job_id" without ever
settling the
original promise, leaving charts and native filters that relied on
`waitForAsyncData`
stuck waiting indefinitely.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38aa5825f23748b3acbb60aa7982d7da&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38aa5825f23748b3acbb60aa7982d7da&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/middleware/asyncEvent.ts
**Line:** 145:149
**Comment:**
*Logic Error: In the non-terminal status branch, the listener is
removed immediately, so if a job emits `pending`/`running` before `done`, the
subsequent terminal event has no listener and the waiting promise can hang
forever. Keep the listener registered for non-terminal states and only clean it
up on terminal states (`done`, `error`) or abort.
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%2F42033&comment_hash=d65780c90e013afe81443baa308baff829c271bd68c865d9d1da33abcdfbf1d6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42033&comment_hash=d65780c90e013afe81443baa308baff829c271bd68c865d9d1da33abcdfbf1d6&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]
