Skip to content

Commit

Permalink
fix(orchestrator): make error handling consistent in backend and UI (#…
Browse files Browse the repository at this point in the history 
…2503)
  • Loading branch information
@batzionb
batzionb authored Nov 7, 2024
1 parent 8cabefd commit 603a162
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 383 deletions.
2 changes: 0 additions & 2 deletions .changeset/fair-tools-teach.md
View file

This file was deleted.

6 changes: 6 additions & 0 deletions .changeset/rotten-foxes-film.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@janus-idp/backstage-plugin-orchestrator-backend": minor
"@janus-idp/backstage-plugin-orchestrator": minor
---

make error handling consistent in backend and UI
2 changes: 0 additions & 2 deletions .changeset/unlucky-peaches-film.md
View file

This file was deleted.

172 changes: 72 additions & 100 deletions plugins/orchestrator-backend/src/service/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,18 @@ export async function createBackendRouter(
throw new Error('next is undefined');
}

routerApi.openApiBackend.handleRequest(req as Request, req, res, next);
return routerApi.openApiBackend.handleRequest(
req as Request,
req,
res,
next,
);
});

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

router.use(middleware.error());

return router;
}

Expand Down Expand Up @@ -297,15 +303,12 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}
await routerApi.v2
return routerApi.v2
.getWorkflowsOverview(buildPagination(req), getRequestFilters(req))
.then(result => res.json(result))
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
next();
next(error);
});
},
);
Expand Down Expand Up @@ -342,19 +345,15 @@ function setupInternalRoutes(
res.status(200).contentType('text/plain').send(result);
} catch (error) {
auditLogRequestError(error, endpointName, endpoint, _req);
res
.status(500)
.contentType('text/plain')
.send((error as Error)?.message || INTERNAL_SERVER_ERROR_MESSAGE);
next();
next(error);
}
},
);

// v2
routerApi.openApiBackend.register(
'executeWorkflow',
async (c, req: express.Request, res: express.Response) => {
async (c, req: express.Request, res: express.Response, next) => {
const workflowId = c.request.params.workflowId as string;
const endpointName = 'executeWorkflow';
const endpoint = `/v2/workflows/${workflowId}/execute`;
Expand Down Expand Up @@ -385,14 +384,12 @@ function setupInternalRoutes(

const executeWorkflowRequestDTO = req.body;

await routerApi.v2
return routerApi.v2
.executeWorkflow(executeWorkflowRequestDTO, workflowId, businessKey)
.then(result => res.status(200).json(result))
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
next(error);
});
},
);
Expand Down Expand Up @@ -423,18 +420,20 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, _req);
}

await routerApi.v2
return routerApi.v2
.getWorkflowOverviewById(workflowId)
.then(result => res.json(result))
.catch(next);
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, _req);
next(error);
});
},
);

// v2
routerApi.openApiBackend.register(
'getWorkflowStatuses',
async (_c, _req: express.Request, res: express.Response) => {
async (_c, _req: express.Request, res: express.Response, next) => {
const endpointName = 'getWorkflowStatuses';
const endpoint = '/v2/workflows/instances/statuses';

Expand All @@ -455,77 +454,59 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, _req);
}
await routerApi.v2
return routerApi.v2
.getWorkflowStatuses()
.then(result => res.status(200).json(result))
.catch((error: { message: string }) => {
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, _req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
next(error);
});
},
);

// v2
routerApi.openApiBackend.register(
'getWorkflowInputSchemaById',
async (c, req: express.Request, res: express.Response) => {
async (c, req: express.Request, res: express.Response, next) => {
const workflowId = c.request.params.workflowId as string;
const instanceId = c.request.query.instanceId as string;
const endpointName = 'getWorkflowInputSchemaById';
const endpoint = `/v2/workflows/${workflowId}/inputSchema`;

auditLogger.auditLog({
eventName: endpointName,
stage: 'start',
status: 'succeeded',
level: 'debug',
request: req,
message: `Received request to '${endpoint}' endpoint`,
});
const decision = await authorize(
req,
orchestratorWorkflowInstanceReadPermission,
permissions,
httpAuth,
);
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}

try {
auditLogger.auditLog({
eventName: endpointName,
stage: 'start',
status: 'succeeded',
level: 'debug',
request: req,
message: `Received request to '${endpoint}' endpoint`,
});
const decision = await authorize(
req,
orchestratorWorkflowInstanceReadPermission,
permissions,
httpAuth,
);
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}

const workflowDefinition =
await services.orchestratorService.fetchWorkflowInfo({
definitionId: workflowId,
cacheHandler: 'throw',
});

if (!workflowDefinition) {
auditLogRequestError(
new Error(`Couldn't fetch workflow definition ${workflowId}`),
endpointName,
endpoint,
req,
throw new Error(
`Failed to fetch workflow info for workflow ${workflowId}`,
);
res
.status(500)
.send(`Couldn't fetch workflow definition ${workflowId}`);
return;
}

const serviceUrl = workflowDefinition.serviceUrl;
if (!serviceUrl) {
auditLogRequestError(
new Error(`Service URL is not defined for workflow ${workflowId}`),
endpointName,
endpoint,
req,
throw new Error(
`Service URL is not defined for workflow ${workflowId}`,
);
res
.status(500)
.send(`Service URL is not defined for workflow ${workflowId}`);
return;
}

const definition =
Expand All @@ -535,20 +516,9 @@ function setupInternalRoutes(
});

if (!definition) {
auditLogRequestError(
new Error(
`Couldn't fetch workflow definition of workflow source ${workflowId}`,
),
endpointName,
endpoint,
req,
throw new Error(
'Failed to fetch workflow definition for workflow ${workflowId}',
);
res
.status(500)
.send(
`Couldn't fetch workflow definition of workflow source ${workflowId}`,
);
return;
}

if (!definition.dataInputSchema) {
Expand All @@ -569,10 +539,14 @@ function setupInternalRoutes(
)
: undefined;

const workflowInfo = await routerApi.v2.getWorkflowInputSchemaById(
workflowId,
serviceUrl,
);
const workflowInfo = await routerApi.v2
.getWorkflowInputSchemaById(workflowId, serviceUrl)
.catch((error: { message: string }) => {
auditLogRequestError(error, endpointName, endpoint, req);
res.status(500).json({
message: error.message || INTERNAL_SERVER_ERROR_MESSAGE,
});
});

if (
!workflowInfo ||
Expand Down Expand Up @@ -602,11 +576,9 @@ function setupInternalRoutes(
inputSchema: workflowInfo.inputSchema,
data: inputData,
});
} catch (error: any) {
auditLogRequestError(error, endpointName, endpoint, req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
} catch (err) {
auditLogRequestError(err, endpointName, endpoint, req);
next(err);
}
},
);
Expand Down Expand Up @@ -637,10 +609,13 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}
await routerApi.v2
return routerApi.v2
.getInstances(buildPagination(req), getRequestFilters(req), workflowId)
.then(result => res.json(result))
.catch(next);
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, req);
next(error);
});
},
);

Expand Down Expand Up @@ -669,10 +644,13 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}
await routerApi.v2
return routerApi.v2
.getInstances(buildPagination(req), getRequestFilters(req))
.then(result => res.json(result))
.catch(next);
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, req);
next(error);
});
},
);

Expand Down Expand Up @@ -706,15 +684,12 @@ function setupInternalRoutes(
c.request,
QUERY_PARAM_INCLUDE_ASSESSMENT,
);
await routerApi.v2
return routerApi.v2
.getInstanceById(instanceId, !!includeAssessment)
.then(result => res.status(200).json(result))
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, _req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
next();
next(error);
});
},
);
Expand Down Expand Up @@ -745,15 +720,12 @@ function setupInternalRoutes(
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, _req);
}
await routerApi.v2
return routerApi.v2
.abortWorkflow(instanceId)
.then(result => res.json(result))
.catch(error => {
auditLogRequestError(error, endpointName, endpoint, _req);
res
.status(500)
.json({ message: error.message || INTERNAL_SERVER_ERROR_MESSAGE });
next();
next(error);
});
},
);
Expand Down
Loading

0 comments on commit 603a162

Please sign in to comment.