diff --git a/.changeset/techdocs-improve-error-reporting.md b/.changeset/techdocs-improve-error-reporting.md new file mode 100644 index 0000000000..fc4e550afd --- /dev/null +++ b/.changeset/techdocs-improve-error-reporting.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Improved error reporting in AzureBlobStorage to surface errors when fetching metadata and uploading files fails. diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 60a705f6a7..6633a71b2c 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -14,6 +14,10 @@ * limitations under the License. */ import fs from 'fs'; +import type { + BlobUploadCommonResponse, + ContainerGetPropertiesResponse, +} from '@azure/storage-blob'; export class BlockBlobClient { private readonly blobName; @@ -22,23 +26,33 @@ export class BlockBlobClient { this.blobName = blobName; } - uploadFile(source: string) { - return new Promise((resolve, reject) => { - if (!fs.existsSync(source)) { - reject(''); - } else { - resolve(''); - } + uploadFile(source: string): Promise { + return Promise.resolve({ + _response: { + request: { + url: `https://example.blob.core.windows.net`, + } as any, + status: 200, + headers: {} as any, + }, }); } exists() { - return new Promise((resolve, reject) => { - if (fs.existsSync(this.blobName)) { - resolve(true); - } else { - reject({ message: 'The object doest not exist !' }); - } + return Promise.resolve(fs.existsSync(this.blobName)); + } +} + +class BlockBlobClientFailUpload extends BlockBlobClient { + uploadFile(source: string): Promise { + return Promise.resolve({ + _response: { + request: { + url: `https://example.blob.core.windows.net`, + } as any, + status: 500, + headers: {} as any, + }, }); } } @@ -50,9 +64,16 @@ export class ContainerClient { this.containerName = containerName; } - getProperties() { - return new Promise(resolve => { - resolve(''); + getProperties(): Promise { + return Promise.resolve({ + _response: { + request: { + url: `https://example.blob.core.windows.net`, + } as any, + status: 200, + headers: {} as any, + parsedHeaders: {}, + }, }); } @@ -61,6 +82,27 @@ export class ContainerClient { } } +class ContainerClientFailGetProperties extends ContainerClient { + getProperties(): Promise { + return Promise.resolve({ + _response: { + request: { + url: `https://example.blob.core.windows.net`, + } as any, + status: 404, + headers: {} as any, + parsedHeaders: {}, + }, + }); + } +} + +class ContainerClientFailUpload extends ContainerClient { + getBlockBlobClient(blobName: string) { + return new BlockBlobClientFailUpload(blobName); + } +} + export class BlobServiceClient { private readonly url; private readonly credential; @@ -71,6 +113,12 @@ export class BlobServiceClient { } getContainerClient(containerName: string) { + if (containerName === 'bad_container') { + return new ContainerClientFailGetProperties(containerName); + } + if (this.credential.accountName === 'failupload') { + return new ContainerClientFailUpload(containerName); + } return new ContainerClient(containerName); } } diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 80f3cb0cf7..62c81df5c4 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -19,6 +19,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { AzureBlobStoragePublish } from './azureBlobStorage'; import { PublisherBase } from './types'; import type { Entity } from '@backstage/catalog-model'; +import type { Logger } from 'winston'; const createMockEntity = (annotations = {}) => { return { @@ -43,33 +44,40 @@ const getEntityRootDir = (entity: Entity) => { return entityRootDir; }; -const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); -jest.spyOn(logger, 'error').mockReturnValue(logger); +function createLogger() { + const logger = getVoidLogger(); + jest.spyOn(logger, 'info').mockReturnValue(logger); + jest.spyOn(logger, 'error').mockReturnValue(logger); + return logger; +} let publisher: PublisherBase; -beforeEach(async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - accountKey: 'accountKey', +describe('publishing with valid credentials', () => { + let logger: Logger; + + beforeEach(async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', + accountKey: 'accountKey', + }, + containerName: 'containerName', }, - containerName: 'containerName', }, }, - }, + }); + + logger = createLogger(); + + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); }); - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); -}); - -describe('AzureBlobStoragePublish', () => { describe('publish', () => { it('should publish a directory', async () => { const entity = createMockEntity(); @@ -116,7 +124,7 @@ describe('AzureBlobStoragePublish', () => { }) .catch(error => expect(error.message).toContain( - 'Unable to upload file(s) to Azure Blob Storage. Error Failed to read template directory: ENOENT, no such file or directory', + 'Unable to upload file(s) to Azure Blob Storage. Failed to read template directory: ENOENT, no such file or directory', ), ); mockFs.restore(); @@ -145,3 +153,92 @@ describe('AzureBlobStoragePublish', () => { }); }); }); + +describe('error reporting', () => { + it('reports an error when unable to read container properties', async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', + }, + containerName: 'bad_container', + }, + }, + }, + }); + + const logger = createLogger(); + + let error; + try { + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(Error); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + `Could not retrieve metadata about the Azure Blob Storage container bad_container.`, + ), + ); + }); + + it('reports an error when bad account credentials', async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'failupload', + accountKey: 'accountKey', + }, + containerName: 'containerName', + }, + }, + }, + }); + + const logger = createLogger(); + + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + mockFs({ + [entityRootDir]: { + 'index.html': '', + }, + }); + + let error; + try { + await publisher.publish({ + entity, + directory: entityRootDir, + }); + } catch (e) { + error = e; + } + + expect(error.message).toContain( + `Unable to upload file(s) to Azure Blob Storage.`, + ); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + `Unable to upload file(s) to Azure Blob Storage. Upload failed for test-namespace/TestKind/test-component-name/index.html with status code 500`, + ), + ); + + mockFs.restore(); + }); +}); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index a02639e63f..7000e9f28c 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -17,7 +17,6 @@ import platformPath from 'path'; import express from 'express'; import { BlobServiceClient, - BlobUploadCommonResponse, StorageSharedKeyCredential, } from '@azure/storage-blob'; import { DefaultAzureCredential } from '@azure/identity'; @@ -79,25 +78,25 @@ export class AzureBlobStoragePublish implements PublisherBase { credential, ); - await storageClient - .getContainerClient(containerName) - .getProperties() - .then(() => { - logger.info( - `Successfully connected to the Azure Blob Storage container ${containerName}.`, - ); - }) - .catch(reason => { - logger.error( - `Could not retrieve metadata about the Azure Blob Storage container ${containerName}. ` + - 'Make sure that the Azure project and container exist and the access key is setup correctly ' + - 'techdocs.publisher.azureBlobStorage.credentials defined in app config has correct permissions. ' + - 'Refer to https://backstage.io/docs/features/techdocs/using-cloud-storage', - ); + try { + const response = await storageClient + .getContainerClient(containerName) + .getProperties(); + + if (response._response.status >= 400) { throw new Error( - `from Azure Blob Storage client library: ${reason.message}`, + `Failed to retrieve metadata from ${response._response.request.url} with status code ${response._response.status}.`, ); - }); + } + } catch (e) { + logger.error( + `Could not retrieve metadata about the Azure Blob Storage container ${containerName}. ` + + 'Make sure that the Azure project and container exist and the access key is setup correctly ' + + 'techdocs.publisher.azureBlobStorage.credentials defined in app config has correct permissions. ' + + 'Refer to https://backstage.io/docs/features/techdocs/using-cloud-storage', + ); + throw new Error(`from Azure Blob Storage client library: ${e.message}`); + } return new AzureBlobStoragePublish(storageClient, containerName, logger); } @@ -122,8 +121,6 @@ export class AzureBlobStoragePublish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); - const uploadPromises: Array> = []; - // Bound the number of concurrent batches. We want a bit of concurrency for // performance reasons, but not so much that we starve the connection pool // or start thrashing. @@ -140,23 +137,43 @@ export class AzureBlobStoragePublish implements PublisherBase { ); // Azure Blob Storage Container file relative path return limiter(async () => { - await uploadPromises.push( - this.storageClient - .getContainerClient(this.containerName) - .getBlockBlobClient(destination) - .uploadFile(filePath), - ); + const response = await this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(destination) + .uploadFile(filePath); + + if (response._response.status >= 400) { + return { + ...response, + error: new Error( + `Upload failed for ${filePath} with status code ${response._response.status}`, + ), + }; + } + return { + ...response, + error: undefined, + }; }); }); - await Promise.all(promises).then(() => { + const responses = await Promise.all(promises); + + const failed = responses.filter(r => r.error); + if (failed.length === 0) { this.logger.info( - `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, + `Successfully uploaded the ${responses.length} generated file(s) for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, ); - }); - return; + } else { + throw new Error( + failed + .map(r => r.error?.message) + .filter(Boolean) + .join(' '), + ); + } } catch (e) { - const errorMessage = `Unable to upload file(s) to Azure Blob Storage. Error ${e.message}`; + const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${e.message}`; this.logger.error(errorMessage); throw new Error(errorMessage); } @@ -241,19 +258,11 @@ export class AzureBlobStoragePublish implements PublisherBase { * A helper function which checks if index.html of an Entity's docs site is available. This * can be used to verify if there are any pre-generated docs available to serve. */ - async hasDocsBeenGenerated(entity: Entity): Promise { - return new Promise(resolve => { - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - this.storageClient - .getContainerClient(this.containerName) - .getBlockBlobClient(`${entityRootDir}/index.html`) - .exists() - .then((response: boolean) => { - resolve(response); - }) - .catch(() => { - resolve(false); - }); - }); + hasDocsBeenGenerated(entity: Entity): Promise { + const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; + return this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(`${entityRootDir}/index.html`) + .exists(); } }