Skip to content

fix: improve terminated stream retry telemetry and bump retries to 3 #2877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wwwillchen merged 1 commit into from
Mar 4, 2026
Merged

fix: improve terminated stream retry telemetry and bump retries to 3 #2877

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ import { getPostCompactionMessages } from "@/ipc/handlers/compaction/compaction_

const logger = log.scope("local_agent_handler");
const PLANNING_QUESTIONNAIRE_TOOL_NAME = "planning_questionnaire";
const MAX_TERMINATED_STREAM_RETRIES = 2;
const MAX_TERMINATED_STREAM_RETRIES = 3;
const STREAM_RETRY_BASE_DELAY_MS = 400;
const STREAM_CONTINUE_MESSAGE =
"[System] Your previous response stream was interrupted by a transient network error. Continue from exactly where you left off and do not repeat text that has already been sent.";
Expand Down Expand Up @@ -1006,6 +1006,7 @@ export async function handleLocalAgentStream(
STREAM_RETRY_BASE_DELAY_MS * terminatedRetryCount;
sendTelemetryEvent("local_agent:terminated_stream_retry", {
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
Expand All @@ -1016,6 +1017,16 @@ export async function handleLocalAgentStream(
await delay(retryDelayMs);
continue;
}
sendTelemetryEvent(
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Inconsistent error handling: in the stream_iteration phase, exhausted retries correctly re-throw the error (throw streamError), but here in the response_finalization phase the error is silently swallowed after sending telemetry. The code falls through to set steps = [] and responseMessages = [], allowing the agent to proceed with an empty response as if the turn succeeded. Add throw err; after the telemetry event to propagate the failure consistently.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts, line 1020:

<comment>Inconsistent error handling: in the `stream_iteration` phase, exhausted retries correctly re-throw the error (`throw streamError`), but here in the `response_finalization` phase the error is silently swallowed after sending telemetry. The code falls through to set `steps = []` and `responseMessages = []`, allowing the agent to proceed with an empty response as if the turn succeeded. Add `throw err;` after the telemetry event to propagate the failure consistently.</comment>

<file context>
@@ -1016,6 +1017,16 @@ export async function handleLocalAgentStream(
               await delay(retryDelayMs);
               continue;
             }
+            sendTelemetryEvent(
+              "local_agent:terminated_stream_retries_exhausted",
+              {
</file context>
Fix with Cubic

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: terminated_stream_retries_exhausted is emitted for non-terminated errors; guard it with isTerminatedStreamError to avoid incorrect telemetry.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts, line 1020:

<comment>`terminated_stream_retries_exhausted` is emitted for non-terminated errors; guard it with `isTerminatedStreamError` to avoid incorrect telemetry.</comment>

<file context>
@@ -1016,6 +1017,16 @@ export async function handleLocalAgentStream(
               await delay(retryDelayMs);
               continue;
             }
+            sendTelemetryEvent(
+              "local_agent:terminated_stream_retries_exhausted",
+              {
</file context>
Fix with Cubic

"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
Comment on lines +1020 to +1029
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Missing isTerminatedStreamError guard causes false retries_exhausted telemetry for non-terminated errors

In the stream_iteration phase, the terminated_stream_retries_exhausted telemetry event is sent unconditionally for any stream error that doesn't qualify for retry. However, shouldRetryTerminatedStreamError returns false for two distinct reasons: (1) the error IS a terminated stream error but retries are exhausted, or (2) the error is NOT a terminated stream error at all (e.g., auth error, rate limit, server error).

Root Cause and Comparison with response_finalization

At local_agent_handler.ts:1020-1029, when shouldRetryTerminatedStreamError returns false, the code unconditionally sends terminated_stream_retries_exhausted before throwing. This means any non-terminated stream error (e.g. a 401 auth error or 429 rate-limit error) will produce a misleading terminated_stream_retries_exhausted telemetry event.

The response_finalization phase at local_agent_handler.ts:1069 correctly guards the same telemetry event with if (isTerminatedStreamError(err)), but this guard is missing in the stream_iteration phase.

Impact: False/misleading telemetry events will pollute the terminated_stream_retries_exhausted metric, making it unreliable for diagnosing actual terminated stream exhaustion.

Suggested change
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
if (isTerminatedStreamError(streamError)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

@cursor cursor bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing guard emits false "retries exhausted" telemetry events

Medium Severity

In the stream_iteration phase, the terminated_stream_retries_exhausted telemetry event fires unconditionally when shouldRetryTerminatedStreamError returns false. Since that function also returns false for errors that aren't terminated stream errors at all, non-terminated stream errors will incorrectly emit this event. The response_finalization phase correctly guards this with if (isTerminatedStreamError(err)), but the stream_iteration phase is missing the equivalent guard.

Additional Locations (1)

Fix in Cursor Fix in Web

throw streamError;
Comment on lines +1020 to 1030
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

terminated_stream_retries_exhausted fired for non-terminated errors

In the stream_iteration phase, the terminated_stream_retries_exhausted event is emitted unconditionally whenever shouldRetryTerminatedStreamError returns false. However, shouldRetryTerminatedStreamError returns false in two distinct cases:

  1. Retries are exhausted (correct — the event is appropriate)
  2. The error is not a terminated stream error (incorrect — the event name is misleading and will produce noisy/wrong telemetry)

The response_finalization phase (added just below this) correctly guards with if (isTerminatedStreamError(err)) before emitting the same event. The stream_iteration path is missing that guard.

Since both streamErrorFromCallback (set in onError) and streamErrorFromIteration (caught from the fullStream async iteration) can be arbitrary errors — not just terminated-stream errors — a non-terminated error will fall through shouldRetryTerminatedStreamError and incorrectly emit this event.

Suggested change
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
throw streamError;
if (isTerminatedStreamError(streamError)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts
Line: 1020-1030

Comment:
**`terminated_stream_retries_exhausted` fired for non-terminated errors**

In the `stream_iteration` phase, the `terminated_stream_retries_exhausted` event is emitted unconditionally whenever `shouldRetryTerminatedStreamError` returns `false`. However, `shouldRetryTerminatedStreamError` returns `false` in **two** distinct cases:

1. Retries are exhausted (correct — the event is appropriate)
2. The error is **not** a terminated stream error (incorrect — the event name is misleading and will produce noisy/wrong telemetry)

The `response_finalization` phase (added just below this) correctly guards with `if (isTerminatedStreamError(err))` before emitting the same event. The `stream_iteration` path is missing that guard.

Since both `streamErrorFromCallback` (set in `onError`) and `streamErrorFromIteration` (caught from the `fullStream` async iteration) can be arbitrary errors — not just terminated-stream errors — a non-terminated error will fall through `shouldRetryTerminatedStreamError` and incorrectly emit this event.

```suggestion
            if (isTerminatedStreamError(streamError)) {
              sendTelemetryEvent(
                "local_agent:terminated_stream_retries_exhausted",
                {
                  chatId: req.chatId,
                  dyadRequestId,
                  retryCount: terminatedRetryCount,
                  error: String(streamError),
                  phase: "stream_iteration",
                },
              );
            }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +1020 to 1030
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM | correctness

Missing isTerminatedStreamError guard on exhausted event (stream_iteration path)

The terminated_stream_retries_exhausted event fires unconditionally here whenever shouldRetryTerminatedStreamError returns false. But that function returns false for three reasons: (1) error is not a terminated stream error, (2) retries are exhausted, or (3) the stream was aborted. Only case (2) should emit this event.

The response_finalization path (line ~1069) correctly wraps its exhausted event with if (isTerminatedStreamError(err)). This path is missing the equivalent guard, which will produce misleading telemetry data.

💡 Suggestion: Add an isTerminatedStreamError(streamError) guard to match the response_finalization pattern:

Suggested change
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
throw streamError;
if (isTerminatedStreamError(streamError)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(streamError),
phase: "stream_iteration",
},
);
}

}

Expand Down Expand Up @@ -1044,6 +1055,7 @@ export async function handleLocalAgentStream(
STREAM_RETRY_BASE_DELAY_MS * terminatedRetryCount;
sendTelemetryEvent("local_agent:terminated_stream_retry", {
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(err),
phase: "response_finalization",
Expand All @@ -1054,6 +1066,18 @@ export async function handleLocalAgentStream(
await delay(retryDelayMs);
continue;
}
if (isTerminatedStreamError(err)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(err),
phase: "response_finalization",
},
);
}
Comment on lines +1069 to +1080
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There's an inconsistency in error handling for exhausted retries between the stream_iteration and response_finalization phases.

In the stream_iteration phase (lines 1020-1030), when retries are exhausted, the streamError is correctly re-thrown, causing the agent turn to fail. However, in this response_finalization phase, the error is not re-thrown after sending the terminated_stream_retries_exhausted event. This causes the error to be silently swallowed, and the agent proceeds with an empty response as if the turn was successful.

To ensure consistent and robust error handling, the error should be re-thrown here to propagate the failure, just as it's done in the other phase.

Suggested change
if (isTerminatedStreamError(err)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(err),
phase: "response_finalization",
},
);
}
if (isTerminatedStreamError(err)) {
sendTelemetryEvent(
"local_agent:terminated_stream_retries_exhausted",
{
chatId: req.chatId,
dyadRequestId,
retryCount: terminatedRetryCount,
error: String(err),
phase: "response_finalization",
},
);
throw err;
}

logger.warn("Failed to retrieve stream response messages:", err);
steps = [];
responseMessages = [];
Expand Down
Loading