IMN-736 - pt 1 - API GW Agreement errors without passthrough / bubble…

…-up (#1009)
pagopa · Oct 2, 2024 · 76e7bfb · 76e7bfb
1 parent 8c71502
commit 76e7bfb
Show file tree

Hide file tree

Showing 10 changed files with 201 additions and 91 deletions.
diff --git 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<AxiosError> =>
-      AxiosLogger.errorLogger(
+    (error: AxiosError): Promise<AxiosError> => {
+      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<typeof AxiosLogger.errorLogger>[0],
         {
           logger: genericLogger.error,
-          prefixText: getPrefix(error.config?.headers, clientName),
+          prefixText,
         }
-      ) as Promise<AxiosError>
+      ) as Promise<AxiosError>;
+    }
   );
 }
diff --git 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
@@ -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<typeof CatchableCodes>;
+const ClientError = z.object({
+  message: z.string(),
+  response: z.object({
+    status: z.coerce.string().pipe(CatchableCodes),
+  }),
+});
+
+type StatusCodeErrorsMap = Record<
+  CatchableCodes,
+  ApiError<ErrorCodes> | undefined
+>;
+export function clientStatusCodeToError(
+  clientResult: unknown,
+  statusCodesErrorMap: Partial<StatusCodeErrorsMap>
+): 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
@@ -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<ErrorCodes> {
-  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<ErrorCodes> {
   return new ApiError({
@@ -123,3 +116,24 @@ export function keyNotFound(kId: string): ApiError<ErrorCodes> {
     title: "Key not found",
   });
 }
+
+export function agreementNotFound(
+  agreementId: agreementApi.Agreement["id"]
+): ApiError<ErrorCodes> {
+  return new ApiError({
+    detail: `Agreement ${agreementId} not found`,
+    code: "agreementNotFound",
+    title: "Agreement not found",
+  });
+}
+
+export function invalidAgreementState(
+  agreementId: agreementApi.Agreement["id"],
+  logger: Logger
+): ApiError<ErrorCodes> {
+  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
@@ -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
@@ -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<ApiGatewayAppContext>,
   queryParams: apiGatewayApi.GetAgreementsQueryParams
 ): Promise<apiGatewayApi.Agreements> {
   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<agreementApi.Agreement> =>
+  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,40 +68,35 @@ export function agreementServiceBuilder(
 ) {
   return {
     getAgreements: async (
-      { logger, headers }: WithLogger<ApiGatewayAppContext>,
+      ctx: WithLogger<ApiGatewayAppContext>,
       queryParams: apiGatewayApi.GetAgreementsQueryParams
     ): Promise<apiGatewayApi.Agreements> => {
       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}`
       );
 
       if (producerId === undefined && consumerId === undefined) {
         throw producerAndConsumerParamMissing();
       }
 
-      return await getAllAgreements(
-        agreementProcessClient,
-        headers,
-        queryParams
-      );
+      return await getAllAgreements(agreementProcessClient, ctx, queryParams);
     },
 
     getAgreementById: async (
       { logger, headers }: WithLogger<ApiGatewayAppContext>,
       agreementId: agreementApi.Agreement["id"]
     ): Promise<apiGatewayApi.Agreement> => {
       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<apiGatewayApi.Attributes> => {
       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<apiGatewayApi.Purposes> => {
       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
@@ -93,20 +93,20 @@ export function purposeServiceBuilder(
     },
 
     getAgreementByPurpose: async (
-      { logger, headers }: WithLogger<ApiGatewayAppContext>,
+      ctx: WithLogger<ApiGatewayAppContext>,
       purposeId: purposeApi.Purpose["id"]
     ): Promise<apiGatewayApi.Agreement> => {
-      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,
         },
       });
 
       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
@@ -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
@@ -14,7 +14,7 @@ const {
 
 export const getAgreementErrorMapper = (error: ApiError<ErrorCodes>): 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<ErrorCodes>): number =>