Skip to content

Commit 603a162

Browse files
batzionbbatzionb
authored
fix(orchestrator): make error handling consistent in backend and UI (#2503)
1 parent 8cabefd commit 603a162

File tree

8 files changed

+152
-383
lines changed

8 files changed

+152
-383
lines changed

‎.changeset/fair-tools-teach.md

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎.changeset/rotten-foxes-film.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@janus-idp/backstage-plugin-orchestrator-backend": minor
3+
"@janus-idp/backstage-plugin-orchestrator": minor
4+
---
5+
6+
make error handling consistent in backend and UI

‎.changeset/unlucky-peaches-film.md

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎plugins/orchestrator-backend/src/service/router.ts

Lines changed: 72 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,18 @@ export async function createBackendRouter(
133133
throw new Error('next is undefined');
134134
}
135135

136-
routerApi.openApiBackend.handleRequest(req as Request, req, res, next);
136+
return routerApi.openApiBackend.handleRequest(
137+
req as Request,
138+
req,
139+
res,
140+
next,
141+
);
137142
});
138143

139144
const middleware = MiddlewareFactory.create({ logger, config });
140145

141146
router.use(middleware.error());
147+
142148
return router;
143149
}
144150

@@ -297,15 +303,12 @@ function setupInternalRoutes(
297303
if (decision.result === AuthorizeResult.DENY) {
298304
manageDenyAuthorization(endpointName, endpoint, req);
299305
}
300-
await routerApi.v2
306+
return routerApi.v2
301307
.getWorkflowsOverview(buildPagination(req), getRequestFilters(req))
302308
.then(result => res.json(result))
303309
.catch(error => {
304310
auditLogRequestError(error, endpointName, endpoint, req);
305-
res
306-
.status(500)
307-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
308-
next();
311+
next(error);
309312
});
310313
},
311314
);
@@ -342,19 +345,15 @@ function setupInternalRoutes(
342345
res.status(200).contentType('text/plain').send(result);
343346
} catch (error) {
344347
auditLogRequestError(error, endpointName, endpoint, _req);
345-
res
346-
.status(500)
347-
.contentType('text/plain')
348-
.send((error as Error)?.message || INTERNAL_SERVER_ERROR_MESSAGE);
349-
next();
348+
next(error);
350349
}
351350
},
352351
);
353352

354353
// v2
355354
routerApi.openApiBackend.register(
356355
'executeWorkflow',
357-
async (c, req: express.Request, res: express.Response) => {
356+
async (c, req: express.Request, res: express.Response, next) => {
358357
const workflowId = c.request.params.workflowId as string;
359358
const endpointName = 'executeWorkflow';
360359
const endpoint = `/v2/workflows/${workflowId}/execute`;
@@ -385,14 +384,12 @@ function setupInternalRoutes(
385384

386385
const executeWorkflowRequestDTO = req.body;
387386

388-
await routerApi.v2
387+
return routerApi.v2
389388
.executeWorkflow(executeWorkflowRequestDTO, workflowId, businessKey)
390389
.then(result => res.status(200).json(result))
391390
.catch(error => {
392391
auditLogRequestError(error, endpointName, endpoint, req);
393-
res
394-
.status(500)
395-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
392+
next(error);
396393
});
397394
},
398395
);
@@ -423,18 +420,20 @@ function setupInternalRoutes(
423420
if (decision.result === AuthorizeResult.DENY) {
424421
manageDenyAuthorization(endpointName, endpoint, _req);
425422
}
426-
427-
await routerApi.v2
423+
return routerApi.v2
428424
.getWorkflowOverviewById(workflowId)
429425
.then(result => res.json(result))
430-
.catch(next);
426+
.catch(error => {
427+
auditLogRequestError(error, endpointName, endpoint, _req);
428+
next(error);
429+
});
431430
},
432431
);
433432

434433
// v2
435434
routerApi.openApiBackend.register(
436435
'getWorkflowStatuses',
437-
async (_c, _req: express.Request, res: express.Response) => {
436+
async (_c, _req: express.Request, res: express.Response, next) => {
438437
const endpointName = 'getWorkflowStatuses';
439438
const endpoint = '/v2/workflows/instances/statuses';
440439

@@ -455,77 +454,59 @@ function setupInternalRoutes(
455454
if (decision.result === AuthorizeResult.DENY) {
456455
manageDenyAuthorization(endpointName, endpoint, _req);
457456
}
458-
await routerApi.v2
457+
return routerApi.v2
459458
.getWorkflowStatuses()
460459
.then(result => res.status(200).json(result))
461-
.catch((error: { message: string }) => {
460+
.catch(error => {
462461
auditLogRequestError(error, endpointName, endpoint, _req);
463-
res
464-
.status(500)
465-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
462+
next(error);
466463
});
467464
},
468465
);
469466

470467
// v2
471468
routerApi.openApiBackend.register(
472469
'getWorkflowInputSchemaById',
473-
async (c, req: express.Request, res: express.Response) => {
470+
async (c, req: express.Request, res: express.Response, next) => {
474471
const workflowId = c.request.params.workflowId as string;
475472
const instanceId = c.request.query.instanceId as string;
476473
const endpointName = 'getWorkflowInputSchemaById';
477474
const endpoint = `/v2/workflows/${workflowId}/inputSchema`;
478-
479-
auditLogger.auditLog({
480-
eventName: endpointName,
481-
stage: 'start',
482-
status: 'succeeded',
483-
level: 'debug',
484-
request: req,
485-
message: `Received request to '${endpoint}' endpoint`,
486-
});
487-
const decision = await authorize(
488-
req,
489-
orchestratorWorkflowInstanceReadPermission,
490-
permissions,
491-
httpAuth,
492-
);
493-
if (decision.result === AuthorizeResult.DENY) {
494-
manageDenyAuthorization(endpointName, endpoint, req);
495-
}
496-
497475
try {
476+
auditLogger.auditLog({
477+
eventName: endpointName,
478+
stage: 'start',
479+
status: 'succeeded',
480+
level: 'debug',
481+
request: req,
482+
message: `Received request to '${endpoint}' endpoint`,
483+
});
484+
const decision = await authorize(
485+
req,
486+
orchestratorWorkflowInstanceReadPermission,
487+
permissions,
488+
httpAuth,
489+
);
490+
if (decision.result === AuthorizeResult.DENY) {
491+
manageDenyAuthorization(endpointName, endpoint, req);
492+
}
493+
498494
const workflowDefinition =
499495
await services.orchestratorService.fetchWorkflowInfo({
500496
definitionId: workflowId,
501497
cacheHandler: 'throw',
502498
});
503499

504500
if (!workflowDefinition) {
505-
auditLogRequestError(
506-
new Error(`Couldn't fetch workflow definition ${workflowId}`),
507-
endpointName,
508-
endpoint,
509-
req,
501+
throw new Error(
502+
`Failed to fetch workflow info for workflow ${workflowId}`,
510503
);
511-
res
512-
.status(500)
513-
.send(`Couldn't fetch workflow definition ${workflowId}`);
514-
return;
515504
}
516-
517505
const serviceUrl = workflowDefinition.serviceUrl;
518506
if (!serviceUrl) {
519-
auditLogRequestError(
520-
new Error(`Service URL is not defined for workflow ${workflowId}`),
521-
endpointName,
522-
endpoint,
523-
req,
507+
throw new Error(
508+
`Service URL is not defined for workflow ${workflowId}`,
524509
);
525-
res
526-
.status(500)
527-
.send(`Service URL is not defined for workflow ${workflowId}`);
528-
return;
529510
}
530511

531512
const definition =
@@ -535,20 +516,9 @@ function setupInternalRoutes(
535516
});
536517

537518
if (!definition) {
538-
auditLogRequestError(
539-
new Error(
540-
`Couldn't fetch workflow definition of workflow source ${workflowId}`,
541-
),
542-
endpointName,
543-
endpoint,
544-
req,
519+
throw new Error(
520+
'Failed to fetch workflow definition for workflow ${workflowId}',
545521
);
546-
res
547-
.status(500)
548-
.send(
549-
`Couldn't fetch workflow definition of workflow source ${workflowId}`,
550-
);
551-
return;
552522
}
553523

554524
if (!definition.dataInputSchema) {
@@ -569,10 +539,14 @@ function setupInternalRoutes(
569539
)
570540
: undefined;
571541

572-
const workflowInfo = await routerApi.v2.getWorkflowInputSchemaById(
573-
workflowId,
574-
serviceUrl,
575-
);
542+
const workflowInfo = await routerApi.v2
543+
.getWorkflowInputSchemaById(workflowId, serviceUrl)
544+
.catch((error: { message: string }) => {
545+
auditLogRequestError(error, endpointName, endpoint, req);
546+
res.status(500).json({
547+
message: error.message || INTERNAL_SERVER_ERROR_MESSAGE,
548+
});
549+
});
576550

577551
if (
578552
!workflowInfo ||
@@ -602,11 +576,9 @@ function setupInternalRoutes(
602576
inputSchema: workflowInfo.inputSchema,
603577
data: inputData,
604578
});
605-
} catch (error: any) {
606-
auditLogRequestError(error, endpointName, endpoint, req);
607-
res
608-
.status(500)
609-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
579+
} catch (err) {
580+
auditLogRequestError(err, endpointName, endpoint, req);
581+
next(err);
610582
}
611583
},
612584
);
@@ -637,10 +609,13 @@ function setupInternalRoutes(
637609
if (decision.result === AuthorizeResult.DENY) {
638610
manageDenyAuthorization(endpointName, endpoint, req);
639611
}
640-
await routerApi.v2
612+
return routerApi.v2
641613
.getInstances(buildPagination(req), getRequestFilters(req), workflowId)
642614
.then(result => res.json(result))
643-
.catch(next);
615+
.catch(error => {
616+
auditLogRequestError(error, endpointName, endpoint, req);
617+
next(error);
618+
});
644619
},
645620
);
646621

@@ -669,10 +644,13 @@ function setupInternalRoutes(
669644
if (decision.result === AuthorizeResult.DENY) {
670645
manageDenyAuthorization(endpointName, endpoint, req);
671646
}
672-
await routerApi.v2
647+
return routerApi.v2
673648
.getInstances(buildPagination(req), getRequestFilters(req))
674649
.then(result => res.json(result))
675-
.catch(next);
650+
.catch(error => {
651+
auditLogRequestError(error, endpointName, endpoint, req);
652+
next(error);
653+
});
676654
},
677655
);
678656

@@ -706,15 +684,12 @@ function setupInternalRoutes(
706684
c.request,
707685
QUERY_PARAM_INCLUDE_ASSESSMENT,
708686
);
709-
await routerApi.v2
687+
return routerApi.v2
710688
.getInstanceById(instanceId, !!includeAssessment)
711689
.then(result => res.status(200).json(result))
712690
.catch(error => {
713691
auditLogRequestError(error, endpointName, endpoint, _req);
714-
res
715-
.status(500)
716-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
717-
next();
692+
next(error);
718693
});
719694
},
720695
);
@@ -745,15 +720,12 @@ function setupInternalRoutes(
745720
if (decision.result === AuthorizeResult.DENY) {
746721
manageDenyAuthorization(endpointName, endpoint, _req);
747722
}
748-
await routerApi.v2
723+
return routerApi.v2
749724
.abortWorkflow(instanceId)
750725
.then(result => res.json(result))
751726
.catch(error => {
752727
auditLogRequestError(error, endpointName, endpoint, _req);
753-
res
754-
.status(500)
755-
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
756-
next();
728+
next(error);
757729
});
758730
},
759731
);

0 commit comments

Comments
 (0)