diff --git a/packages/api-clients/src/axiosLogInterceptors.ts b/packages/api-clients/src/axiosLogInterceptors.ts index 70149dedd7..604e6d2743 100644 --- a/packages/api-clients/src/axiosLogInterceptors.ts +++ b/packages/api-clients/src/axiosLogInterceptors.ts @@ -32,13 +32,21 @@ export function configureAxiosLogInterceptors( prefixText: getPrefix(response.config.headers, clientName), } ) as AxiosResponse, - (error: AxiosError): Promise => - AxiosLogger.errorLogger( + (error: AxiosError): Promise => { + const prefixText = getPrefix(error.config?.headers, clientName); + if ("errors" in error && Array.isArray(error.errors)) { + error.errors.forEach((err) => + genericLogger.error(`[${prefixText}] ${err}`) + ); + } + + return AxiosLogger.errorLogger( error as Parameters[0], { logger: genericLogger.error, - prefixText: getPrefix(error.config?.headers, clientName), + prefixText, } - ) as Promise + ) as Promise; + } ); } diff --git a/packages/api-gateway/src/api/agreementApiConverter.ts b/packages/api-gateway/src/api/agreementApiConverter.ts index 31afb8b3f5..fe8321ee94 100644 --- a/packages/api-gateway/src/api/agreementApiConverter.ts +++ b/packages/api-gateway/src/api/agreementApiConverter.ts @@ -1,4 +1,5 @@ import { agreementApi, apiGatewayApi } from "pagopa-interop-api-clients"; +import { Logger } from "pagopa-interop-commons"; import { assertAgreementStateNotDraft } from "../services/validators.js"; const allowedAgreementStates: apiGatewayApi.AgreementState[] = [ @@ -10,9 +11,10 @@ const allowedAgreementStates: apiGatewayApi.AgreementState[] = [ ]; export function toApiGatewayAgreementIfNotDraft( - agreement: agreementApi.Agreement + agreement: agreementApi.Agreement, + logger: Logger ): apiGatewayApi.Agreement { - assertAgreementStateNotDraft(agreement.state, agreement.id); + assertAgreementStateNotDraft(agreement.state, agreement.id, logger); return { id: agreement.id, diff --git a/packages/api-gateway/src/clients/catchClientError.ts b/packages/api-gateway/src/clients/catchClientError.ts new file mode 100644 index 0000000000..dab276cef4 --- /dev/null +++ b/packages/api-gateway/src/clients/catchClientError.ts @@ -0,0 +1,40 @@ +import { z } from "zod"; +import { ApiError, genericError } from "pagopa-interop-models"; +import { ErrorCodes } from "../models/errors.js"; + +const CatchableCodes = z.enum(["404", "403"]); +type CatchableCodes = z.infer; +const ClientError = z.object({ + message: z.string(), + response: z.object({ + status: z.coerce.string().pipe(CatchableCodes), + }), +}); + +type StatusCodeErrorsMap = Record< + CatchableCodes, + ApiError | undefined +>; +export function clientStatusCodeToError( + clientResult: unknown, + statusCodesErrorMap: Partial +): Error { + const clientError = ClientError.safeParse(clientResult); + + if (clientError.success) { + const error = statusCodesErrorMap[clientError.data.response.status]; + if (error) { + return error; + } + } + + if (clientResult instanceof Error) { + return clientResult; + // Returning the original error as it is, + // so that it can be thrown, handled and logged + } + + return genericError( + `Unexpected error response from API Client: ${JSON.stringify(clientResult)}` + ); +} diff --git a/packages/api-gateway/src/models/errors.ts b/packages/api-gateway/src/models/errors.ts index 9c34b9d41d..0a73860bfc 100644 --- a/packages/api-gateway/src/models/errors.ts +++ b/packages/api-gateway/src/models/errors.ts @@ -4,10 +4,11 @@ import { catalogApi, purposeApi, } from "pagopa-interop-api-clients"; +import { Logger } from "pagopa-interop-commons"; import { ApiError, makeApiProblemBuilder } from "pagopa-interop-models"; export const errorCodes = { - invalidAgreementState: "0001", + agreementNotFound: "0001", producerAndConsumerParamMissing: "0002", missingActivePurposeVersion: "0003", activeAgreementByEserviceAndConsumerNotFound: "0004", @@ -21,18 +22,10 @@ export const errorCodes = { export type ErrorCodes = keyof typeof errorCodes; -export const makeApiProblem = makeApiProblemBuilder(errorCodes); - -export function invalidAgreementState( - state: agreementApi.AgreementState, - agreementId: agreementApi.Agreement["id"] -): ApiError { - return new ApiError({ - detail: `Cannot retrieve agreement in ${state} state - id: ${agreementId}`, - code: "invalidAgreementState", - title: "Invalid agreement state", - }); -} +export const makeApiProblem = makeApiProblemBuilder( + errorCodes, + false // API Gateway shall not let Problem errors from other services to pass through +); export function producerAndConsumerParamMissing(): ApiError { return new ApiError({ @@ -123,3 +116,24 @@ export function keyNotFound(kId: string): ApiError { title: "Key not found", }); } + +export function agreementNotFound( + agreementId: agreementApi.Agreement["id"] +): ApiError { + return new ApiError({ + detail: `Agreement ${agreementId} not found`, + code: "agreementNotFound", + title: "Agreement not found", + }); +} + +export function invalidAgreementState( + agreementId: agreementApi.Agreement["id"], + logger: Logger +): ApiError { + const error = agreementNotFound(agreementId); + logger.warn( + `Root cause for Error "${error.title}": cannot retrieve agreement in DRAFT state` + ); + return error; +} diff --git a/packages/api-gateway/src/routers/apiGatewayRouter.ts b/packages/api-gateway/src/routers/apiGatewayRouter.ts index 77a3d17a98..9640547270 100644 --- a/packages/api-gateway/src/routers/apiGatewayRouter.ts +++ b/packages/api-gateway/src/routers/apiGatewayRouter.ts @@ -151,7 +151,11 @@ const apiGatewayRouter = ( return res.status(200).send(attributes); } catch (error) { - const errorRes = makeApiProblem(error, emptyErrorMapper, ctx.logger); + const errorRes = makeApiProblem( + error, + getAgreementErrorMapper, + ctx.logger + ); return res.status(errorRes.status).send(errorRes); } } @@ -170,7 +174,11 @@ const apiGatewayRouter = ( return res.status(200).send(purposes); } catch (error) { - const errorRes = makeApiProblem(error, emptyErrorMapper, ctx.logger); + const errorRes = makeApiProblem( + error, + getAgreementErrorMapper, + ctx.logger + ); return res.status(errorRes.status).send(errorRes); } } diff --git a/packages/api-gateway/src/services/agreementService.ts b/packages/api-gateway/src/services/agreementService.ts index 67c37fbf2a..d1ec7c712f 100644 --- a/packages/api-gateway/src/services/agreementService.ts +++ b/packages/api-gateway/src/services/agreementService.ts @@ -7,14 +7,18 @@ import { } from "../clients/clientsProvider.js"; import { ApiGatewayAppContext } from "../utilities/context.js"; import { toApiGatewayAgreementIfNotDraft } from "../api/agreementApiConverter.js"; -import { producerAndConsumerParamMissing } from "../models/errors.js"; +import { + agreementNotFound, + producerAndConsumerParamMissing, +} from "../models/errors.js"; import { toAgreementProcessGetAgreementsQueryParams } from "../api/agreementApiConverter.js"; import { toApiGatewayAgreementAttributes } from "../api/attributeApiConverter.js"; +import { clientStatusCodeToError } from "../clients/catchClientError.js"; import { getAllPurposes } from "./purposeService.js"; export async function getAllAgreements( agreementProcessClient: AgreementProcessClient, - headers: ApiGatewayAppContext["headers"], + { headers, logger }: WithLogger, queryParams: apiGatewayApi.GetAgreementsQueryParams ): Promise { const getAgreementsQueryParams = @@ -31,9 +35,31 @@ export async function getAllAgreements( }, }) ); - return { agreements: agreements.map(toApiGatewayAgreementIfNotDraft) }; + return { + agreements: agreements.map((agreement) => + toApiGatewayAgreementIfNotDraft(agreement, logger) + ), + }; } +const retrieveAgreement = ( + agreementProcessClient: AgreementProcessClient, + headers: ApiGatewayAppContext["headers"], + agreementId: agreementApi.Agreement["id"] +): Promise => + agreementProcessClient + .getAgreementById({ + headers, + params: { + agreementId, + }, + }) + .catch((res) => { + throw clientStatusCodeToError(res, { + 404: agreementNotFound(agreementId), + }); + }); + // eslint-disable-next-line @typescript-eslint/explicit-function-return-type export function agreementServiceBuilder( agreementProcessClient: AgreementProcessClient, @@ -42,13 +68,13 @@ export function agreementServiceBuilder( ) { return { getAgreements: async ( - { logger, headers }: WithLogger, + ctx: WithLogger, queryParams: apiGatewayApi.GetAgreementsQueryParams ): Promise => { const { producerId, consumerId, eserviceId, descriptorId, states } = queryParams; - logger.info( + ctx.logger.info( `Retrieving agreements for producerId ${producerId} consumerId ${consumerId} eServiceId ${eserviceId} descriptorId ${descriptorId} states ${states}` ); @@ -56,11 +82,7 @@ export function agreementServiceBuilder( throw producerAndConsumerParamMissing(); } - return await getAllAgreements( - agreementProcessClient, - headers, - queryParams - ); + return await getAllAgreements(agreementProcessClient, ctx, queryParams); }, getAgreementById: async ( @@ -68,14 +90,13 @@ export function agreementServiceBuilder( agreementId: agreementApi.Agreement["id"] ): Promise => { logger.info(`Retrieving agreement by id = ${agreementId}`); - const agreement = await agreementProcessClient.getAgreementById({ + const agreement = await retrieveAgreement( + agreementProcessClient, headers, - params: { - agreementId, - }, - }); + agreementId + ); - return toApiGatewayAgreementIfNotDraft(agreement); + return toApiGatewayAgreementIfNotDraft(agreement, logger); }, getAgreementAttributes: async ( @@ -84,12 +105,11 @@ export function agreementServiceBuilder( ): Promise => { logger.info(`Retrieving Attributes for Agreement ${agreementId}`); - const agreement = await agreementProcessClient.getAgreementById({ + const agreement = await retrieveAgreement( + agreementProcessClient, headers, - params: { - agreementId, - }, - }); + agreementId + ); const tenant = await tenantProcessClient.tenant.getTenant({ headers, @@ -107,12 +127,11 @@ export function agreementServiceBuilder( ): Promise => { logger.info(`Retrieving Purposes for Agreement ${agreementId}`); - const agreement = await agreementProcessClient.getAgreementById({ + const agreement = await retrieveAgreement( + agreementProcessClient, headers, - params: { - agreementId, - }, - }); + agreementId + ); return await getAllPurposes(purposeProcessClient, headers, { eserviceId: agreement.eserviceId, diff --git a/packages/api-gateway/src/services/purposeService.ts b/packages/api-gateway/src/services/purposeService.ts index 805bc86129..abc2b4899b 100644 --- a/packages/api-gateway/src/services/purposeService.ts +++ b/packages/api-gateway/src/services/purposeService.ts @@ -93,12 +93,12 @@ export function purposeServiceBuilder( }, getAgreementByPurpose: async ( - { logger, headers }: WithLogger, + ctx: WithLogger, purposeId: purposeApi.Purpose["id"] ): Promise => { - logger.info(`Retrieving agreement by purpose ${purposeId}`); + ctx.logger.info(`Retrieving agreement by purpose ${purposeId}`); const purpose = await purposeProcessClient.getPurpose({ - headers, + headers: ctx.headers, params: { id: purposeId, }, @@ -106,7 +106,7 @@ export function purposeServiceBuilder( const { agreements } = await getAllAgreements( agreementProcessClient, - headers, + ctx, { consumerId: purpose.consumerId, eserviceId: purpose.eserviceId, diff --git a/packages/api-gateway/src/services/validators.ts b/packages/api-gateway/src/services/validators.ts index 6352d0963d..feadc86b73 100644 --- a/packages/api-gateway/src/services/validators.ts +++ b/packages/api-gateway/src/services/validators.ts @@ -6,6 +6,7 @@ import { purposeApi, } from "pagopa-interop-api-clients"; import { operationForbidden, TenantId } from "pagopa-interop-models"; +import { Logger } from "pagopa-interop-commons"; import { activeAgreementByEserviceAndConsumerNotFound, attributeNotFoundInRegistry, @@ -19,10 +20,11 @@ import { NonDraftCatalogApiDescriptor } from "../api/catalogApiConverter.js"; export function assertAgreementStateNotDraft( agreementState: agreementApi.AgreementState, - agreementId: agreementApi.Agreement["id"] + agreementId: agreementApi.Agreement["id"], + logger: Logger ): asserts agreementState is apiGatewayApi.AgreementState { if (agreementState === agreementApi.AgreementState.Values.DRAFT) { - throw invalidAgreementState(agreementState, agreementId); + throw invalidAgreementState(agreementId, logger); } } diff --git a/packages/api-gateway/src/utilities/errorMappers.ts b/packages/api-gateway/src/utilities/errorMappers.ts index c87b28623e..8d818ad83c 100644 --- a/packages/api-gateway/src/utilities/errorMappers.ts +++ b/packages/api-gateway/src/utilities/errorMappers.ts @@ -14,7 +14,7 @@ const { export const getAgreementErrorMapper = (error: ApiError): number => match(error.code) - .with("invalidAgreementState", () => HTTP_STATUS_NOT_FOUND) + .with("agreementNotFound", () => HTTP_STATUS_NOT_FOUND) .otherwise(() => HTTP_STATUS_INTERNAL_SERVER_ERROR); export const getAgreementsErrorMapper = (error: ApiError): number => diff --git a/packages/models/src/errors.ts b/packages/models/src/errors.ts index 9e7107570d..069281e784 100644 --- a/packages/models/src/errors.ts +++ b/packages/models/src/errors.ts @@ -82,9 +82,12 @@ const makeProblemLogString = ( return `- title: ${problem.title} - detail: ${problem.detail} - errors: ${errorsString} - original error: ${originalError}`; }; -export function makeApiProblemBuilder(errors: { - [K in T]: string; -}): MakeApiProblemFn { +export function makeApiProblemBuilder( + errors: { + [K in T]: string; + }, + problemErrorsPassthrough: boolean = true +): MakeApiProblemFn { const allErrors = { ...errorCodes, ...errors }; return (error, httpMapper, logger, operationalMsg) => { const makeProblem = ( @@ -102,46 +105,60 @@ export function makeApiProblemBuilder(errors: { })), }); - return ( - match(error) - .with(P.instanceOf(ApiError), (error) => { - const problem = makeProblem(httpMapper(error), error); - logger.warn(makeProblemLogString(problem, error)); - return problem; - }) - // this case is to allow a passthrough of PROBLEM errors in the BFF - .with( - { - response: { + const genericProblem = makeProblem(500, genericError("Unexpected error")); + + return match(error) + .with(P.instanceOf(ApiError), (error) => { + const problem = makeProblem(httpMapper(error), error); + logger.warn(makeProblemLogString(problem, error)); + return problem; + }) + .with( + /* this case is to allow a passthrough of Problem errors, so that + services that call other interop services can forward Problem errors + as they are, without the need to explicitly handle them */ + { + response: { + status: P.number, + data: { + type: "about:blank", + title: P.string, status: P.number, - data: { - type: "about:blank", - title: P.string, - status: P.number, + detail: P.string, + errors: P.array({ + code: P.string, detail: P.string, - errors: P.array({ - code: P.string, - detail: P.string, - }), - correlationId: P.string.optional(), - }, + }), + correlationId: P.string.optional(), }, }, - (e) => { - const receivedProblem: Problem = e.response.data; - if (operationalMsg) { - logger.warn(operationalMsg); - } + }, + (e) => { + const receivedProblem: Problem = e.response.data; + if (operationalMsg) { + logger.warn(operationalMsg); + } + + if (problemErrorsPassthrough) { logger.warn(makeProblemLogString(receivedProblem, error)); - return e.response.data; + return receivedProblem; + } else { + logger.warn( + makeProblemLogString( + genericProblem, + `${receivedProblem.title}, code ${ + receivedProblem.errors.at(0)?.code + }, ${receivedProblem.errors.at(0)?.detail}` + ) + ); + return genericProblem; } - ) - .otherwise((error: unknown) => { - const problem = makeProblem(500, genericError("Unexpected error")); - logger.error(makeProblemLogString(problem, error)); - return problem; - }) - ); + } + ) + .otherwise((error: unknown): Problem => { + logger.error(makeProblemLogString(genericProblem, error)); + return genericProblem; + }); }; }