Skip to content

Commit

Permalink
Beter 404 for plots-tables (#140)
Browse files Browse the repository at this point in the history
* Beter 404 for plots-tables

* Friendlier logging of expected errors

* Broken if omg

* Better logging

* Log hook execution

* Wrong condition, again :-(

* Take care of falsy parsedMessage
  • Loading branch information
xverges authored Jun 2, 2021
1 parent 6c72660 commit 37ced64
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/api/general-services/pipeline-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const getPipelineStatus = async (experimentId, processName) => {
/* eslint-enable no-await-in-loop */

completedSteps = getStepsFromExecutionHistory(events);
logger.log(`ExecutionHistory for ARN ${executionArn}: ${events.length} events, ${completedSteps.length} completed steps`);
logger.log(`ExecutionHistory(${processName}) for ARN ${executionArn}: ${events.length} events, ${completedSteps.length} completed steps`);
}

const response = {
Expand Down
24 changes: 13 additions & 11 deletions src/api/routes/gem2s.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,19 @@ module.exports = {
}

const { io, parsedMessage } = result;

try {
await gem2sResponse(io, parsedMessage);
} catch (e) {
logger.error(
'Pipeline response handler failed with error: ', e,
);

AWSXRay.getSegment().addError(e);
res.status(200).send('nok');
return;
const isSnsNotification = parsedMessage !== undefined;
if (isSnsNotification) {
try {
await gem2sResponse(io, parsedMessage);
} catch (e) {
logger.error(
'gem2s pipeline response handler failed with error: ', e,
);

AWSXRay.getSegment().addError(e);
res.status(200).send('nok');
return;
}
}

res.status(200).send('ok');
Expand Down
22 changes: 12 additions & 10 deletions src/api/routes/pipelines.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,19 @@ module.exports = {
}

const { io, parsedMessage } = result;
const isSnsNotification = parsedMessage !== undefined;
if (isSnsNotification) {
try {
await pipelineResponse(io, parsedMessage);
} catch (e) {
logger.error(
'qc pipeline response handler failed with error: ', e,
);

try {
await pipelineResponse(io, parsedMessage);
} catch (e) {
logger.error(
'Pipeline response handler failed with error: ', e,
);

AWSXRay.getSegment().addError(e);
res.status(200).send('nok');
return;
AWSXRay.getSegment().addError(e);
res.status(200).send('nok');
return;
}
}

res.status(200).send('ok');
Expand Down
4 changes: 3 additions & 1 deletion src/api/routes/plots-tables.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const PlotsTablesService = require('../route-services/plots-tables');
const { NotFoundError } = require('../../utils/responses');


const plotsTablesService = new PlotsTablesService();

Expand All @@ -16,7 +18,7 @@ module.exports = {
.then((response) => res.json(response))
.catch((e) => {
if (e.message.includes('not found')) {
res.status(404).send('');
throw (new NotFoundError('Plot not found'));
} else {
throw e;
}
Expand Down
6 changes: 6 additions & 0 deletions src/specs/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,12 @@ paths:
application/json:
schema:
$ref: ./models/HTTPError.v1.yaml
'404':
description: Experiment/plot not found.
content:
application/json:
schema:
$ref: ./models/HTTPError.v1.yaml
'200':
description: OK
content:
Expand Down
3 changes: 3 additions & 0 deletions src/utils/hookRunner.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const logger = require('./logging');

class hookRunner {
constructor() {
this.hooks = {};
Expand Down Expand Up @@ -27,6 +29,7 @@ class hookRunner {
// eslint-disable-next-line no-await-in-loop
this.results[taskName].push(await this.hooks[taskName][idx](payload));
}
logger.log(`Completed ${this.results[taskName].length} hooks for pipeline task ${taskName}`);

return this.results;
}
Expand Down
2 changes: 2 additions & 0 deletions src/utils/parse-sns-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const parseSNSMessage = async (req) => {
logger.error('Error parsing message:', e);
throw e;
}
} else {
logger.log(`SNS message is of type "${msg.Type}", ignoring it`);
}

return {};
Expand Down

0 comments on commit 37ced64

Please sign in to comment.