Skip to content

fix: prevent silent failures when releasing stream reader locks #3397

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

Closed
meenaksh06 wants to merge 1 commit into from
Closed

fix: prevent silent failures when releasing stream reader locks #3397

Show file tree
Hide file tree
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
7 changes: 6 additions & 1 deletion packages/core/src/v3/realtimeStreams/streamInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,5 +153,10 @@ async function* streamToAsyncIterator<T>(stream: ReadableStream<T>): AsyncIterab
function safeReleaseLock(reader: ReadableStreamDefaultReader<any>) {
try {
reader.releaseLock();
} catch (error) {}
} catch (error) {
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Comment on lines +157 to +160
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🔴 Undefined debug variable in safeReleaseLock causes ReferenceError, defeating error safety

safeReleaseLock is a standalone module-level function, not a class method. The new code references debug in the catch block (if (debug)), but debug is never declared, imported, or passed as a parameter in this file. If reader.releaseLock() throws, the catch block will itself throw a ReferenceError: debug is not defined, converting a safely-caught error into an uncaught crash — the exact opposite of the function's purpose.

Suggested change
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
if (typeof debug !== "undefined" && debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Open in Devin Review

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

}
}
7 changes: 6 additions & 1 deletion packages/core/src/v3/realtimeStreams/streamsWriterV1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,5 +464,10 @@ async function* streamToAsyncIterator<T>(stream: ReadableStream<T>): AsyncIterab
function safeReleaseLock(reader: ReadableStreamDefaultReader<any>) {
try {
reader.releaseLock();
} catch (error) {}
} catch (error) {
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Comment on lines +468 to +471
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🔴 Undefined debug variable in safeReleaseLock causes ReferenceError, defeating error safety

Same issue as in streamInstance.ts. The standalone safeReleaseLock function references debug which is not in scope. The StreamsWriterV1 class has no debug property at all, and even if it did, this is not a class method so this.debug is inaccessible. If releaseLock() throws, the catch handler will crash with ReferenceError.

Suggested change
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
if (typeof debug !== "undefined" && debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Open in Devin Review

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

}
}
Comment on lines 464 to 473
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🚩 Design consideration: safeReleaseLock has no access to debug context

Even after fixing the ReferenceError, the underlying design issue remains: safeReleaseLock is a standalone function with no way to know whether debug mode is enabled. In streamsWriterV2.ts, the class has this.debug (streamsWriterV2.ts:52), and in streamInstance.ts, the options have debug (streamInstance.ts:17), but streamsWriterV1.ts has no debug concept at all. A clean fix would either: (a) pass debug as a parameter to safeReleaseLock, (b) always log the warning unconditionally (it's in a catch block so it only fires on errors), or (c) extract it to a shared util that accepts a debug flag.

Open in Devin Review

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

7 changes: 6 additions & 1 deletion packages/core/src/v3/realtimeStreams/streamsWriterV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,5 +212,10 @@ async function* streamToAsyncIterator<T>(stream: ReadableStream<T>): AsyncIterab
function safeReleaseLock(reader: ReadableStreamDefaultReader<any>) {
try {
reader.releaseLock();
} catch (error) {}
} catch (error) {
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Comment on lines +216 to +219
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🔴 Undefined debug variable in safeReleaseLock causes ReferenceError, defeating error safety

Same issue as the other two files. The StreamsWriterV2 class has private readonly debug: boolean at line 52, but the standalone safeReleaseLock function at module scope cannot access class instance properties. The reference to bare debug at line 216 will throw ReferenceError if releaseLock() fails.

Suggested change
if (debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
if (typeof debug !== "undefined" && debug) {
// fallback if no logger available
console.warn("Failed to release stream reader lock", error);
}
Open in Devin Review

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

}
}