s3: fix path double-deinit when operation throws after store creation - #30567
Closed
robobun wants to merge 6 commits into
Closed
s3: fix path double-deinit when operation throws after store creation#30567robobun wants to merge 6 commits into
robobun wants to merge 6 commits into
Claude / Claude Code Review
completed
May 12, 2026 in 29m 23s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/bun/s3/s3-write-throwing-data.test.ts:34-44 |
type-getter test throws pre-initS3, not post-initS3 as intended |
Annotations
Check warning on line 44 in test/js/bun/s3/s3-write-throwing-data.test.ts
claude / Claude Code Review
type-getter test throws pre-initS3, not post-initS3 as intended
This test was added to cover the "throw **after** `initS3`" path, but `getCredentialsWithOptions` (credentials_jsc.zig:203) also reads `opts.type` and runs *before* `Blob.Store.initS3` in both `constructS3FileWithS3Credentials` and `constructS3FileWithS3CredentialsAndOptions` — so with `{ get type() { throw } }` the throw fires there and `initS3`/`path.* = empty` is never reached. To actually exercise the post-`initS3` ownership-transfer path here, make the getter succeed on the first read and t
Loading