diff --git a/.changeset/stale-chairs-look.md b/.changeset/stale-chairs-look.md new file mode 100644 index 0000000000..75a277901a --- /dev/null +++ b/.changeset/stale-chairs-look.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-node': patch +--- + +Update Azure file retrieval logic from storing file in buffer array to piping to res for better memory efficiency. diff --git a/plugins/techdocs-node/src/stages/publish/azureBlobStorage.test.ts b/plugins/techdocs-node/src/stages/publish/azureBlobStorage.test.ts index 83e707fd54..3b5e8d5632 100644 --- a/plugins/techdocs-node/src/stages/publish/azureBlobStorage.test.ts +++ b/plugins/techdocs-node/src/stages/publish/azureBlobStorage.test.ts @@ -21,7 +21,6 @@ import request from 'supertest'; import path from 'path'; import fs from 'fs-extra'; import { AzureBlobStoragePublish } from './azureBlobStorage'; -import { EventEmitter } from 'events'; import { BlobUploadCommonResponse, ContainerGetPropertiesResponse, @@ -62,20 +61,13 @@ jest.mock('@azure/storage-blob', () => { } download() { - const emitter = new EventEmitter(); - setTimeout(() => { - if (fs.pathExistsSync(mockDir.resolve(this.blobName))) { - emitter.emit('data', fs.readFileSync(mockDir.resolve(this.blobName))); - emitter.emit('end'); - } else { - emitter.emit( - 'error', - new Error(`The file ${this.blobName} does not exist!`), - ); - } - }, 0); + if (!fs.pathExistsSync(mockDir.resolve(this.blobName))) { + return Promise.reject( + new Error(`The file ${this.blobName} does not exist!`), + ); + } return Promise.resolve({ - readableStreamBody: emitter, + readableStreamBody: fs.createReadStream(mockDir.resolve(this.blobName)), }); } } @@ -211,6 +203,7 @@ jest.mock('@azure/storage-blob', () => { __esModule: true, BlobServiceClient, StorageSharedKeyCredential, + BlockBlobClient, }; }); @@ -572,5 +565,31 @@ describe('AzureBlobStoragePublish', () => { 'File Not Found', ); }); + + it('should handle stream pipe errors', async () => { + // Get BlockBlobClient from the mock module and replace the download method with a failing one + const { BlockBlobClient } = jest.requireMock('@azure/storage-blob'); + const originalDownload = BlockBlobClient.prototype.download; + BlockBlobClient.prototype.download = function () { + return Promise.resolve({ + readableStreamBody: { + pipe: () => { + throw new Error('Pipe operation failed'); + }, + }, + }); + }; + + const response = await request(app).get( + `/${entityTripletPath}/index.html`, + ); + + BlockBlobClient.prototype.download = originalDownload; + + expect(response.status).toBe(404); + expect(Buffer.from(response.text).toString('utf8')).toEqual( + 'File Not Found', + ); + }); }); }); diff --git a/plugins/techdocs-node/src/stages/publish/azureBlobStorage.ts b/plugins/techdocs-node/src/stages/publish/azureBlobStorage.ts index 863a4fc533..9d257312dd 100644 --- a/plugins/techdocs-node/src/stages/publish/azureBlobStorage.ts +++ b/plugins/techdocs-node/src/stages/publish/azureBlobStorage.ts @@ -286,32 +286,6 @@ export class AzureBlobStoragePublish implements PublisherBase { return { objects }; } - private download(containerName: string, blobPath: string): Promise { - return new Promise((resolve, reject) => { - const fileStreamChunks: Array = []; - this.storageClient - .getContainerClient(containerName) - .getBlockBlobClient(blobPath) - .download() - .then(res => { - const body = res.readableStreamBody; - if (!body) { - reject(new Error(`Unable to parse the response data`)); - return; - } - body - .on('error', reject) - .on('data', chunk => { - fileStreamChunks.push(chunk); - }) - .on('end', () => { - resolve(Buffer.concat(fileStreamChunks)); - }); - }) - .catch(reject); - }); - } - async fetchTechDocsMetadata( entityName: CompoundEntityRef, ): Promise { @@ -321,10 +295,30 @@ export class AzureBlobStoragePublish implements PublisherBase { : lowerCaseEntityTriplet(entityTriplet); try { - const techdocsMetadataJson = await this.download( - this.containerName, - `${entityRootDir}/techdocs_metadata.json`, - ); + const techdocsMetadataJson = await new Promise((resolve, reject) => { + const fileStreamChunks: Array = []; + this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(`${entityRootDir}/techdocs_metadata.json`) + .download() + .then(res => { + const body = res.readableStreamBody; + if (!body) { + reject(new Error(`Unable to parse the response data`)); + return; + } + body + .on('error', reject) + .on('data', chunk => { + fileStreamChunks.push(chunk); + }) + .on('end', () => { + resolve(Buffer.concat(fileStreamChunks)); + }); + }) + .catch(reject); + }); + if (!techdocsMetadataJson) { throw new Error( `Unable to parse the techdocs metadata file ${entityRootDir}/techdocs_metadata.json.`, @@ -356,21 +350,33 @@ export class AzureBlobStoragePublish implements PublisherBase { const fileExtension = platformPath.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); - this.download(this.containerName, filePath) - .then(fileContent => { - // Inject response headers + const blobClient = this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(filePath); + + blobClient + .download() + .then(downloadRes => { + if (!downloadRes.readableStreamBody) { + throw new Error('Unable to parse the response data'); + } + // Set headers after confirming file exists and can be downloaded for (const [headerKey, headerValue] of Object.entries( responseHeaders, )) { res.setHeader(headerKey, headerValue); } - res.send(fileContent); + downloadRes.readableStreamBody.pipe(res); }) .catch(e => { this.logger.warn( `TechDocs Azure router failed to serve content from container ${this.containerName} at path ${filePath}: ${e.message}`, ); - res.status(404).send('File Not Found'); + if (!res.headersSent) { + res.status(404).send('File Not Found'); + } else { + res.destroy(); + } }); }; }