From 593db0864d6c7be2f58fb58e781a907dfa853295 Mon Sep 17 00:00:00 2001 From: Crevil Date: Tue, 28 Sep 2021 14:04:37 +0200 Subject: [PATCH] Expand Techdocs not found error handling Currently if a document is requested that is found found a generic log line containing only the error from the provider is logged. {"type":"plugin","plugin":"techdocs","message":"The specified key does not exist.","level":"warn","service":"backstage"} This gives very little indication as to what and where something failed. This change expands the log lines to include more details about what is being attempted. It also removes the propagation of the raw error message to the client as there might be information here that is not meant to be disclosed. Tests are added for each provider. A bug in the Azure Blob storage provider was found where the download method would reject its promise but the express handler would not detect this and therefore not write a response. Signed-off-by: Crevil --- .changeset/gorgeous-monkeys-remain.md | 6 ++++++ .../src/stages/publish/awsS3.test.ts | 11 +++++++++++ .../techdocs-common/src/stages/publish/awsS3.ts | 6 ++++-- .../src/stages/publish/azureBlobStorage.test.ts | 11 +++++++++++ .../src/stages/publish/azureBlobStorage.ts | 14 ++++++++------ .../src/stages/publish/googleStorage.test.ts | 11 +++++++++++ .../src/stages/publish/googleStorage.ts | 6 ++++-- .../src/stages/publish/openStackSwift.test.ts | 16 ++++++++++++++++ .../src/stages/publish/openStackSwift.ts | 9 +++++++-- 9 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 .changeset/gorgeous-monkeys-remain.md diff --git a/.changeset/gorgeous-monkeys-remain.md b/.changeset/gorgeous-monkeys-remain.md new file mode 100644 index 0000000000..f9bbb669f2 --- /dev/null +++ b/.changeset/gorgeous-monkeys-remain.md @@ -0,0 +1,6 @@ +--- +'@backstage/techdocs-common': patch +--- + +Add more context to techdocs log lines when files are not found along with +ensuring that the routers return 404 with a descriptive message. diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index dbc81a25b4..35bc80438b 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -352,5 +352,16 @@ describe('AwsS3Publish', () => { 'content-type': 'text/plain; charset=utf-8', }); }); + + it('should return 404 if file is not found', async () => { + const response = await request(app).get( + `/${entityTripletPath}/not-found.html`, + ); + expect(response.status).toBe(404); + + expect(Buffer.from(response.text).toString('utf8')).toEqual( + 'File Not Found', + ); + }); }); }); diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 85857929d5..f2e1b867ec 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -351,8 +351,10 @@ export class AwsS3Publish implements PublisherBase { res.send(await streamToBuffer(stream)); } catch (err) { - this.logger.warn(err.message); - res.status(404).json(err.message); + this.logger.warn( + `TechDocs S3 router failed to serve static files from bucket ${this.bucketName} at key ${filePath}: ${err.message}`, + ); + res.status(404).send('File Not Found'); } }; } diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 914cc24986..cace6caae8 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -375,5 +375,16 @@ describe('AzureBlobStoragePublish', () => { 'content-type': 'text/plain; charset=utf-8', }); }); + + it('should return 404 if file is not found', async () => { + const response = await request(app).get( + `/${entityTripletPath}/not-found.html`, + ); + expect(response.status).toBe(404); + + expect(Buffer.from(response.text).toString('utf8')).toEqual( + 'File Not Found', + ); + }); }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 4dfd8ba2f5..2d3df4e6b4 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -328,8 +328,8 @@ export class AzureBlobStoragePublish implements PublisherBase { const fileExtension = platformPath.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); - try { - this.download(this.containerName, filePath).then(fileContent => { + this.download(this.containerName, filePath) + .then(fileContent => { // Inject response headers for (const [headerKey, headerValue] of Object.entries( responseHeaders, @@ -337,11 +337,13 @@ export class AzureBlobStoragePublish implements PublisherBase { res.setHeader(headerKey, headerValue); } res.send(fileContent); + }) + .catch(e => { + this.logger.error( + `TechDocs Azure router failed to serve content from container ${this.containerName} at path ${filePath}: ${e.message}`, + ); + res.status(404).send('File Not Found'); }); - } catch (e) { - this.logger.error(e.message); - res.status(404).json(e.message); - } }; } diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 4247442c54..0190128e6c 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -351,5 +351,16 @@ describe('GoogleGCSPublish', () => { 'content-type': 'text/plain; charset=utf-8', }); }); + + it('should return 404 if file is not found', async () => { + const response = await request(app).get( + `/${entityTripletPath}/not-found.html`, + ); + expect(response.status).toBe(404); + + expect(Buffer.from(response.text).toString('utf8')).toEqual( + 'File Not Found', + ); + }); }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 1f519829f2..ba5bec8752 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -261,10 +261,12 @@ export class GoogleGCSPublish implements PublisherBase { res.writeHead(200, responseHeaders); }) .on('error', err => { - this.logger.warn(err.message); + this.logger.error( + `TechDocs Google GCS router failed to serve content from bucket ${this.bucketName} at path ${filePath}: ${err.message}`, + ); // Send a 404 with a meaningful message if possible. if (!res.headersSent) { - res.status(404).send(err.message); + res.status(404).send('File Not Found'); } else { res.destroy(); } diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index 84007c0f5b..8dde687997 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -364,5 +364,21 @@ describe('OpenStackSwiftPublish', () => { 'content-type': 'text/plain; charset=utf-8', }); }); + + it('should return 404 if file is not found', async () => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + const response = await request(app).get( + `/${namespace}/${kind}/${name}/not-found.html`, + ); + expect(response.status).toBe(404); + + expect(Buffer.from(response.text).toString('utf8')).toEqual( + 'File Not Found', + ); + }); }); }); diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.ts index 2902d7384f..b5111ec2fb 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.ts @@ -245,10 +245,15 @@ export class OpenStackSwiftPublish implements PublisherBase { res.send(await streamToBuffer(stream)); } catch (err) { - this.logger.warn(err.message); - res.status(404).send(err.message); + this.logger.error( + `TechDocs OpenStack swift router failed to serve content from container ${this.containerName} at path ${filePath}: ${err.message}`, + ); + res.status(404).send('File Not Found'); } } else { + this.logger.error( + `TechDocs OpenStack swift router failed to serve content from container ${this.containerName} at path ${filePath}: Not found`, + ); res.status(404).send('File Not Found'); } };