codeant-ai-for-open-source[bot] commented on code in PR #42012:
URL: https://github.com/apache/superset/pull/42012#discussion_r3578000148
##########
superset-frontend/src/middleware/asyncEvent.test.ts:
##########
@@ -173,6 +173,237 @@ describe('asyncEvent middleware', () => {
expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength(1);
});
+ test('backs off exponentially when polling requests keep failing', async
() => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // first poll fires after the configured delay and fails
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // next poll is delayed by 2x the configured delay, so nothing yet
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ // after the second failure the delay grows to 4x
+ await jest.advanceTimersByTimeAsync(
+ 3 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(3);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('resumes the configured polling delay after a successful poll', async
() => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // two failed polls: 1x delay, then 2x delay
+ await jest.advanceTimersByTimeAsync(
+ 3 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ // subsequent polls succeed, resetting the backoff
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, {
+ status: 200,
+ body: { result: [] },
+ });
+
+ // third poll fires 4x delay after the second failure and succeeds
+ await jest.advanceTimersByTimeAsync(
+ 4 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // polling is back to the configured delay
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('caps the polling backoff delay at 60 seconds', async () => {
+ const MAX_ERROR_POLLING_DELAY_MS = 60000;
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // first poll fires after the configured delay and fails
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // walk the uncapped backoff: after failure N the next delay is
+ // 2^N times the configured delay, which stays below the cap through
+ // failure 10 (50ms * 2^10 = 51.2s)
+ for (let failures = 1; failures <= 10; failures += 1) {
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY * 2 ** failures,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(
+ failures + 1,
+ );
+ }
+
+ // after failure 11 the uncapped delay would be 102.4s, so the cap
+ // takes over: no poll just before the 60s mark...
+ await jest.advanceTimersByTimeAsync(MAX_ERROR_POLLING_DELAY_MS - 1);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(11);
+
+ // ...and the next poll fires exactly at 60s
+ await jest.advanceTimersByTimeAsync(1);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(12);
+
+ // additional failures remain capped at 60s
+ await jest.advanceTimersByTimeAsync(MAX_ERROR_POLLING_DELAY_MS);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(13);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('does not start a second loop when re-initialized during an in-flight
poll', async () => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ let resolveFetch: (response: any) => void = () => {};
Review Comment:
**Suggestion:** Replace the `any` in this function type with a concrete
response type (for example the specific fetch-mock response shape used in this
test) to preserve type safety. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new test code explicitly annotates the callback parameter as `any`,
which matches the llmcfg-no-any-types rule violation. The file is a TypeScript
test file and the `any` is present in the final code, so this is a real
violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ebf2fe547ce642ce8efdd4ea830471ce&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=ebf2fe547ce642ce8efdd4ea830471ce&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.test.ts
**Line:** 317:317
**Comment:**
*Custom Rule: Replace the `any` in this function type with a concrete
response type (for example the specific fetch-mock response shape used in this
test) to preserve type safety.
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%2F42012&comment_hash=6e0bc6e86e249f94f7096d4997fc54d665121d3777b6f7cf6d42de7feabf01bc&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42012&comment_hash=6e0bc6e86e249f94f7096d4997fc54d665121d3777b6f7cf6d42de7feabf01bc&reaction=dislike'>๐</a>
##########
superset-frontend/src/middleware/asyncEvent.test.ts:
##########
@@ -173,6 +173,237 @@ describe('asyncEvent middleware', () => {
expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength(1);
});
+ test('backs off exponentially when polling requests keep failing', async
() => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // first poll fires after the configured delay and fails
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // next poll is delayed by 2x the configured delay, so nothing yet
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ // after the second failure the delay grows to 4x
+ await jest.advanceTimersByTimeAsync(
+ 3 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(3);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('resumes the configured polling delay after a successful poll', async
() => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // two failed polls: 1x delay, then 2x delay
+ await jest.advanceTimersByTimeAsync(
+ 3 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+
+ // subsequent polls succeed, resetting the backoff
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, {
+ status: 200,
+ body: { result: [] },
+ });
+
+ // third poll fires 4x delay after the second failure and succeeds
+ await jest.advanceTimersByTimeAsync(
+ 4 * config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // polling is back to the configured delay
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('caps the polling backoff delay at 60 seconds', async () => {
+ const MAX_ERROR_POLLING_DELAY_MS = 60000;
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, { status: 403 });
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // first poll fires after the configured delay and fails
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // walk the uncapped backoff: after failure N the next delay is
+ // 2^N times the configured delay, which stays below the cap through
+ // failure 10 (50ms * 2^10 = 51.2s)
+ for (let failures = 1; failures <= 10; failures += 1) {
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY * 2 ** failures,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(
+ failures + 1,
+ );
+ }
+
+ // after failure 11 the uncapped delay would be 102.4s, so the cap
+ // takes over: no poll just before the 60s mark...
+ await jest.advanceTimersByTimeAsync(MAX_ERROR_POLLING_DELAY_MS - 1);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(11);
+
+ // ...and the next poll fires exactly at 60s
+ await jest.advanceTimersByTimeAsync(1);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(12);
+
+ // additional failures remain capped at 60s
+ await jest.advanceTimersByTimeAsync(MAX_ERROR_POLLING_DELAY_MS);
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(13);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('does not start a second loop when re-initialized during an in-flight
poll', async () => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ let resolveFetch: (response: any) => void = () => {};
+ fetchMock.get(
+ EVENTS_ENDPOINT,
+ new Promise(resolve => {
+ resolveFetch = resolve;
+ }),
+ );
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+
+ // first poll fires and stays in-flight
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ // re-init while that fetch is pending, then let it resolve; the
+ // stale invocation must not schedule a second loop
+ asyncEvent.init(config);
+ asyncEvent.waitForAsyncData(asyncPendingEvent).catch(() => {});
+ resolveFetch({ status: 200, body: { result: [] } });
+ await jest.advanceTimersByTimeAsync(0);
+
+ fetchMock.clearHistory().removeRoutes();
+ fetchMock.get(EVENTS_ENDPOINT, {
+ status: 200,
+ body: { result: [] },
+ });
+
+ // exactly one poll per delay from here on
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1);
+
+ await jest.advanceTimersByTimeAsync(
+ config.GLOBAL_ASYNC_QUERIES_POLLING_DELAY,
+ );
+ expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(2);
+ } finally {
+ jest.useRealTimers();
+ }
+ });
+
+ test('does not resume polling when re-initialized with the feature
disabled during an in-flight poll', async () => {
+ // stop the real-timer polling loop started by beforeEach before
+ // switching to fake timers, so all polls run on the fake clock
+ mockedIsFeatureEnabled.mockReturnValueOnce(false);
+ asyncEvent.init(config);
+ jest.useFakeTimers();
+ try {
+ fetchMock.clearHistory().removeRoutes();
+ let resolveFetch: (response: any) => void = () => {};
Review Comment:
**Suggestion:** Change this `any` parameter annotation to a concrete typed
payload so the callback contract remains explicit and type-checked.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The final file still contains a TypeScript annotation using `any` in newly
added test code. That directly violates the no-any-types rule, so the
suggestion is valid and verifiable.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=afe143c85e944879b13aef4499077514&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=afe143c85e944879b13aef4499077514&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.test.ts
**Line:** 369:369
**Comment:**
*Custom Rule: Change this `any` parameter annotation to a concrete
typed payload so the callback contract remains explicit and type-checked.
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%2F42012&comment_hash=8f440f817ae6da32949d81388b740bb75386146715647bfbd9a3950cdc907685&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42012&comment_hash=8f440f817ae6da32949d81388b740bb75386146715647bfbd9a3950cdc907685&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]
