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 <bjoern.soerensen@gmail.com>
This commit is contained in:
Crevil
2021-09-28 14:04:37 +02:00
parent c3c93e0909
commit 593db0864d
9 changed files with 78 additions and 12 deletions
+6
View File
@@ -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.
@@ -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',
);
});
});
});
@@ -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');
}
};
}
@@ -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',
);
});
});
});
@@ -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);
}
};
}
@@ -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',
);
});
});
});
@@ -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();
}
@@ -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',
);
});
});
});
@@ -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');
}
};