feat: add ArtifactsError type definitions#6710
Conversation
Add typed error surface for the Artifacts binding: - ArtifactsErrorCode: union of all error code strings - ArtifactsError: extends Error with readonly code, numericCode - @throws annotations on all Artifacts and ArtifactsRepo methods Follows the same interface-extends-Error pattern used by StreamError, ImagesError, and MediaError. Note: generated-snapshot/ needs regeneration via `just generate-types`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6710 +/- ##
=======================================
Coverage 66.62% 66.62%
=======================================
Files 402 402
Lines 115914 115914
Branches 19425 19425
=======================================
Hits 77226 77226
Misses 27095 27095
Partials 11593 11593 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/bonk review this PR |
There was a problem hiding this comment.
AI-generated review (by OpenCode). May contain inaccuracies — please verify.
Summary: The types look well-structured and follow existing Cloudflare binding error patterns. A few items to address:
- The
generated-snapshot/files should not be hand-edited — CI should regenerate them. The manual edits will likely be overwritten and may cause the snapshot CI check to fail untiljust generate-typesis run. - Several error codes in
ArtifactsErrorCodeare never referenced in any@throwsannotation. Either they can be thrown (and should be documented) or they're unused. - Minor: the
import()method documentsINVALID_INPUTfor bad URLs, but a more specificINVALID_URLcode exists in the union and seems like a better fit.
|
Review posted successfully. Here's a summary of the feedback I left on PR #6710: 3 inline comments:
|
|
Hi @dmmulroy. Can you address/resolve the bonk review comments and then I'll give it another look before approving? Thanks. |
Merging this PR will not alter performance
Comparing Footnotes
|
Addressed 🫡 h/t @dinasaur404 for help on it |
|
/bonk review the PR |
What
Add
ArtifactsErrorandArtifactsErrorCodeTypeScript type definitions to the Artifacts binding.Changes
types/defines/artifacts.d.ts:ArtifactsErrorCode— union of all error code strings (ALREADY_EXISTS,NOT_FOUND,INVALID_INPUT, etc.)ArtifactsError— extendsErrorwithreadonly code: ArtifactsErrorCodeandreadonly numericCode: number@throws {ArtifactsError}annotations on allArtifactsandArtifactsRepomethodsFollows the same
interface extends Errorpattern used byStreamError,ImagesError, andMediaError.Snapshot
generated-snapshot/needs regeneration — CI will flag the diff and produce the updated snapshot artifact. I was unable to runjust generate-typeslocally due to a bazel cache issue unrelated to this change.