codeant-ai-for-open-source[bot] commented on code in PR #42305:
URL: https://github.com/apache/superset/pull/42305#discussion_r3658163882
##########
superset-frontend/src/middleware/asyncEvent.ts:
##########
@@ -161,11 +171,13 @@ export const waitForAsyncData = async (
}
};
- // When the caller aborts (chart superseded/unmounted), stop listening so
the
- // listener and its retained closure don't leak and keep the poller busy.
+ // When the caller aborts (Stop pressed, chart superseded/unmounted), stop
+ // listening so the listener and its retained closure don't leak, and ask
the
+ // server to cancel the job so it stops consuming warehouse resources.
if (signal) {
onAbort = () => {
cleanup();
+ cancelAsyncJob(jobId);
reject(new DOMException('Aborted', 'AbortError'));
};
Review Comment:
**Suggestion:** Aborting while the terminal event is downloading cached data
is not converted into the expected `AbortError`. The terminal-event handler
removes `onAbort` before awaiting `fetchCachedData`, while `fetchCachedData`
catches the aborted request and turns it into a normal client-error result, so
the chart can dispatch a failure instead of `chartUpdateStopped` and the
server-side cancellation is not requested. Preserve abort semantics for the
cached-data fetch and cancel the job when that fetch is aborted. [race
condition]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Stop can produce chart-load failure state.
- ⚠️ `chartUpdateStopped` is skipped during cached-result races.
- ⚠️ Server cancellation is skipped after terminal-event cleanup.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start an async query through `waitForAsyncData()` in
`superset-frontend/src/middleware/asyncEvent.ts:116-187`, passing the chart
request's
`AbortSignal`.
2. Let the async event listener at
`superset-frontend/src/middleware/asyncEvent.ts:139-161` receive a terminal
`JOB_STATUS.DONE` event; its terminal handling performs cleanup before
awaiting the
cached-result request.
3. While `fetchCachedData()` at
`superset-frontend/src/middleware/asyncEvent.ts:87-105` is
awaiting `SupersetClient.get()` for `asyncEvent.result_url`, press Stop or
otherwise abort
the request signal.
4. Because cleanup already removed the abort listener, the handler at
`superset-frontend/src/middleware/asyncEvent.ts:178-182` does not run; the
aborted GET is
caught by `fetchCachedData()` at lines 99-102 and returned as `{ status:
'error', data:
... }`, so `waitForAsyncData()` resolves with a normal failure instead of
rejecting with
`AbortError`. The chart therefore follows failure handling rather than the
existing
stopped-chart handling, and `cancelAsyncJob()` at lines 107-114 is not
requested during
this race.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d4ffa0e39b8e4de4a6e011dcbee846a6&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=d4ffa0e39b8e4de4a6e011dcbee846a6&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:** 178:182
**Comment:**
*Race Condition: Aborting while the terminal event is downloading
cached data is not converted into the expected `AbortError`. The terminal-event
handler removes `onAbort` before awaiting `fetchCachedData`, while
`fetchCachedData` catches the aborted request and turns it into a normal
client-error result, so the chart can dispatch a failure instead of
`chartUpdateStopped` and the server-side cancellation is not requested.
Preserve abort semantics for the cached-data fetch and cancel the job when that
fetch is aborted.
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%2F42305&comment_hash=5d97fc6ab6a683be30efaa65a380f392483f030e1380e60e9a1398dbee658d2a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42305&comment_hash=5d97fc6ab6a683be30efaa65a380f392483f030e1380e60e9a1398dbee658d2a&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]
