@tus/azure-store: fix metadata parsing bug#790
Conversation
🦋 Changeset detectedLatest commit: a224979 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
WalkthroughChanged AzureStore.getMetadata to parse raw Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/azure-store/src/index.ts (3)
118-121: Make parsing resilient to unexpected shapes and parse errorsDefend against cases where upload.metadata might already be an object (older records/tests) or an invalid string. Fallback to {} instead of throwing.
Apply:
- upload.metadata = upload.metadata ? Metadata.parse(upload.metadata) : {} + try { + if (typeof upload.metadata === 'string' && upload.metadata.length > 0) { + upload.metadata = Metadata.parse(upload.metadata) + } else if (upload.metadata && typeof upload.metadata === 'object') { + // already parsed; keep as-is + } else { + upload.metadata = {} + } + } catch (e) { + log('Invalid TUS metadata string; defaulting to {}', e as Error) + upload.metadata = {} + }
71-73: Typos in docstring“metada” → “metadata”, “everytime” → “every time”.
- * Saves upload metadata to blob metada. Also upload metadata - * gets saved in local cache as well to avoid calling azure server everytime. + * Saves upload metadata to blob metadata. Also saves it + * in local cache to avoid calling the Azure service every time.
145-156: Optional: add a targeted test to prevent regressionsConsider adding an AzureStore metadata round‑trip test (including non‑ASCII and null values) similar to packages/utils/src/test/stores.ts to lock this fix.
I can draft a minimal test that creates an upload with metadata {filename: '世界.pdf', is_confidential: null}, persists it, reads it back, and deepStrictEquals the metadata. Want me to open a follow‑up PR with that?
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/azure-store/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/azure-store/src/index.ts (1)
packages/gcs-store/src/index.ts (1)
upload(172-180)
🔇 Additional comments (1)
packages/azure-store/src/index.ts (1)
118-121: Good fix: parse the raw TUS metadata string instead of JSON-stringifying itThis removes the extra quoting that broke Metadata.parse and restores round‑trip symmetry with saveMetadata’s Metadata.stringify.
|
CI is failing |
|
ping |
Removes serialization that wraps the metadata (that's already a string) in quotes.
These quote caused
Metadata.parseto fail with the following error: "Metadata string is not valid"Closes #791