chore(deps): migrate from azure-storage to @azure/storage-* SDK
#1410
Conversation
JamieMagee
changed the title
azure-storage to @azure/storage-* SDK
There was a problem hiding this comment.
Pull request overview
This PR migrates from the deprecated azure-storage package to the modern Azure SDK packages (@azure/storage-blob v12.30.0 and @azure/storage-queue v12.29.0). The migration replaces callback-based APIs with native Promises, uses async iterators for blob listing, and updates error handling to use HTTP status codes instead of error codes.
Changes:
- Replace
azure-storagewith@azure/storage-bloband@azure/storage-queuepackages - Convert all blob and queue operations from callback-based to Promise-based async/await patterns
- Update error handling to check
error.statusCodeinstead oferror.codefor 404 checks - Implement async iterators for blob listing operations replacing continuation token-based pagination
- Update all test mocks to reflect new SDK interfaces with proper async iterator and stream handling
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds @azure/storage-blob and @azure/storage-queue dependencies, removes azure-storage, adds xml2js |
| package-lock.json | Updates dependency tree with new Azure SDK packages and their dependencies |
| providers/stores/abstractAzblobStore.js | Migrates base store class to use BlobServiceClient with Promise-based APIs and async iterators |
| providers/stores/azblobHarvestStore.js | Updates harvest store with async iterators and Promise-based blob operations |
| providers/stores/azblobDefinitionStore.js | Migrates definition store to use new SDK with updated error handling (statusCode vs code) |
| providers/stores/azblobAttachmentStore.js | Updates attachment store with new blob client and stream-to-string helper |
| providers/queueing/azureStorageQueue.js | Migrates queue service to QueueServiceClient with receiveMessages API |
| test/providers/store/azblobHarvest.js | Updates test mocks to simulate new SDK's containerClient and async iterators |
| test/providers/store/azblobDefinition.js | Refactors test mocks for new blob client methods with proper async patterns |
| test/providers/store/azblobAttachment.js | Updates attachment store tests with new getBlobClient mock interface |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JamieMagee
force-pushed
the
jamiemagee/migrate-azure-storage-sdk
branch
from
6d356b4 to
1cef4ef
Compare
…zure/storage-queue Replace deprecated azure-storage package with the modern Azure SDK: - @azure/storage-blob for blob storage operations - @azure/storage-queue for queue operations The new SDK uses native Promises, async iterators, and built-in retry policies.
JamieMagee
force-pushed
the
jamiemagee/migrate-azure-storage-sdk
branch
from
1cef4ef to
12a753d
Compare
JamieMagee
left a comment
JamieMagee
left a comment
There was a problem hiding this comment.
Found a potential bug with double base64 encoding in the queue implementation.
| async queue(message) { | ||
| await promisify(this.queueService.createMessage).bind(this.queueService)(this.options.queueName, message) | ||
| // The new SDK expects base64 encoded messages by default for compatibility | ||
| const encodedMessage = Buffer.from(message).toString('base64') | ||
| await this.queueClient.sendMessage(encodedMessage) |
There was a problem hiding this comment.
This manually base64-encodes the message before calling sendMessage(), but the new @azure/storage-queue SDK accepts plain strings and handles encoding internally. The old azure-storage SDK worked the same way.
This causes double-encoding:
- Manual base64 encoding here
- Azure Queue Storage encodes the message again when storing it
During a rolling deployment where old and new code versions run simultaneously, messages queued by the new code will be double-encoded. Old consumers will decode once and get base64 gibberish instead of valid JSON.
The fix is to remove the manual encoding:
async queue(message) {
await this.queueClient.sendMessage(message)
}The new @azure/storage-queue SDK does not automatically base64-encode messages, so we should not decode them either. Messages are now sent and received as plain JSON strings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
providers/queueing/azureStorageQueue.js:68
- Similar to the dequeue() method, this code doesn't handle base64-encoded messages from the old SDK. If there are existing messages in the queue that were encoded with the old SDK, they won't be properly decoded. Consider adding backward compatibility to handle both base64-encoded (old) and plain text (new) messages.
if (message.dequeueCount <= 5) {
results.push({
original: message,
data: JSON.parse(message.messageText)
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| downloadResponse.readableStreamBody.pipe(stream) | ||
| downloadResponse.readableStreamBody.on('end', () => resolve(downloadResponse)) | ||
| downloadResponse.readableStreamBody.on('error', reject) |
There was a problem hiding this comment.
The stream piping logic doesn't handle the 'close' event of the destination stream. If the destination stream is closed prematurely (e.g., due to an error on the write side), this could lead to unhandled promise rejections or resource leaks. Consider adding an event handler for the 'close' event on the destination stream to properly clean up and reject the promise if the stream closes unexpectedly.
| downloadResponse.readableStreamBody.pipe(stream) | |
| downloadResponse.readableStreamBody.on('end', () => resolve(downloadResponse)) | |
| downloadResponse.readableStreamBody.on('error', reject) | |
| const source = downloadResponse.readableStreamBody | |
| const dest = stream | |
| let settled = false | |
| const cleanup = () => { | |
| source.removeListener('end', onSourceEnd) | |
| source.removeListener('error', onSourceError) | |
| if (dest) { | |
| dest.removeListener('error', onDestError) | |
| dest.removeListener('close', onDestClose) | |
| } | |
| } | |
| const onSourceEnd = () => { | |
| if (settled) return | |
| settled = true | |
| cleanup() | |
| resolve(downloadResponse) | |
| } | |
| const onSourceError = err => { | |
| if (settled) return | |
| settled = true | |
| cleanup() | |
| reject(err) | |
| } | |
| const onDestError = err => { | |
| if (settled) return | |
| settled = true | |
| cleanup() | |
| reject(err) | |
| } | |
| const onDestClose = () => { | |
| if (settled) return | |
| settled = true | |
| cleanup() | |
| reject(new Error('Destination stream closed before source stream ended')) | |
| } | |
| source.on('end', onSourceEnd) | |
| source.on('error', onSourceError) | |
| if (dest) { | |
| dest.on('error', onDestError) | |
| dest.on('close', onDestClose) | |
| source.pipe(dest) | |
| } else { | |
| // If no destination stream is provided, just drain the source | |
| source.on('data', () => {}) | |
| } |
| if (message.dequeueCount <= 5) { | ||
| return { | ||
| original: message, | ||
| data: JSON.parse(message.messageText) | ||
| } |
There was a problem hiding this comment.
The old azure-storage SDK automatically base64-encoded queue messages, but the new @azure/storage-queue SDK does not. The code now directly parses message.messageText without base64 decoding. This is a breaking change if there are existing messages in the queue that were encoded with the old SDK, as they won't be decodable. Consider adding backward compatibility to handle both base64-encoded (old) and plain text (new) messages during the migration period, or ensure all queues are drained before deploying this change.
This issue also appears in the following locations of the same file:
- line 64
| async _streamToString(readableStream) { | ||
| /** @type {Buffer[]} */ | ||
| const chunks = [] | ||
| for await (const chunk of readableStream) { | ||
| chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) | ||
| } | ||
| return Buffer.concat(chunks).toString('utf8') | ||
| } |
There was a problem hiding this comment.
The _streamToString helper method is duplicated in this file and in abstractAzblobStore.js. Since AzBlobAttachmentStore doesn't extend AbstractAzBlobStore, this duplication is necessary. However, consider extracting this into a shared utility module to maintain DRY principles and ensure consistent behavior across all storage implementations.
| const response = await this.queueClient.receiveMessages({ numberOfMessages: 1 }) | ||
| if (!response.receivedMessageItems || response.receivedMessageItems.length === 0) return null | ||
|
|
||
| const message = response.receivedMessageItems[0] | ||
| if (message.dequeueCount <= 5) { | ||
| return { | ||
| original: message, | ||
| data: JSON.parse(message.messageText) | ||
| } | ||
| } | ||
| await this.delete({ original: message }) | ||
| return this.dequeue() |
There was a problem hiding this comment.
The recursive call to dequeue() when a message has exceeded the dequeue count could potentially cause a stack overflow if there are many consecutive messages with high dequeue counts. While unlikely in practice, consider using a loop instead of recursion to handle this scenario more safely.
| const response = await this.queueClient.receiveMessages({ numberOfMessages: 1 }) | |
| if (!response.receivedMessageItems || response.receivedMessageItems.length === 0) return null | |
| const message = response.receivedMessageItems[0] | |
| if (message.dequeueCount <= 5) { | |
| return { | |
| original: message, | |
| data: JSON.parse(message.messageText) | |
| } | |
| } | |
| await this.delete({ original: message }) | |
| return this.dequeue() | |
| // Use a loop instead of recursion to avoid potential stack overflows when | |
| // many consecutive messages exceed the dequeueCount threshold. | |
| while (true) { | |
| const response = await this.queueClient.receiveMessages({ numberOfMessages: 1 }) | |
| if (!response.receivedMessageItems || response.receivedMessageItems.length === 0) return null | |
| const message = response.receivedMessageItems[0] | |
| if (message.dequeueCount <= 5) { | |
| return { | |
| original: message, | |
| data: JSON.parse(message.messageText) | |
| } | |
| } | |
| await this.delete({ original: message }) | |
| } |
Summary
Migrate from the deprecated
azure-storagepackage to the modern Azure SDK packages:@azure/storage-blobfor blob storage@azure/storage-queuefor queue storageChanges
promisify()wrappers (new SDK is Promise-native)withFilter()retry policy (built into new SDK)References
This is part of a series of PRs to address deprecated packages in the project.
Stack
@types/winston#1411vso-node-apidependency #1412