close stream before releasing lock

[deathbyknowledge]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: cd65c3e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@cloudflare/sandbox	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/sandbox-sdk/@cloudflare/sandbox@254

commit: cd65c3e

[github-actions]

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-254-5535f48

Version: 0.0.0-pr-254-5535f48

You can use this Docker image with the preview package from this PR.

[claude]

Claude Code Review

The implementation is correct - calling cancel() before releaseLock() properly terminates HTTP connections when streams are broken early. However, needs a changeset before merge.

Issues:

1. Missing changeset - The existing .changeset/tough-buttons-arrive.md is for "Update dependencies", not this fix. Need a new changeset:

   ---
   '@cloudflare/sandbox': patch
   ---
   
   Fix stream cleanup by calling cancel() before releaseLock()

2. Comment inconsistency - sse-parser.ts:75 has shorter comment than the other two files. Use the detailed version everywhere for clarity.

Optional: Consider adding a test case for early stream termination (breaking from for await loop).

Verdict: Needs changeset before merge.

[claude]

Claude Code Review

The stream cancellation fix is correct. Calling reader.cancel() before reader.releaseLock() properly terminates HTTP connections per Web Streams API spec, which prevents connection leaks when breaking from generators early.

One issue remains from previous review: sse-parser.ts still uses empty catch block instead of the detailed comment used in the other two files. This inconsistency should be fixed.

Verdict: Approve after fixing comment consistency in sse-parser.ts:77