From ad6fadb015a37d3811041f2c58dd3a3681411b4e Mon Sep 17 00:00:00 2001 From: barsa Date: Mon, 15 Dec 2025 13:32:42 +0900 Subject: [PATCH] Implement DomainHttpException for stable error handling and refactor exception filter - Introduced `DomainHttpException` to standardize HTTP error responses with explicit domain error codes. - Refactored `UnifiedExceptionFilter` to extract detailed exception information, including optional explicit error codes. - Updated `WhmcsErrorHandlerService` to utilize `DomainHttpException` for better error normalization and handling. - Removed deprecated error pattern matching logic to enforce explicit error codes in the system. - Enhanced logging in `WhmcsHttpClientService` to differentiate between expected business outcomes and actual errors. --- .../src/core/http/domain-http.exception.ts | 21 +++ apps/bff/src/core/http/exception.filter.ts | 62 +++++-- .../services/whmcs-error-handler.service.ts | 161 +++++++----------- .../services/whmcs-http-client.service.ts | 6 +- apps/portal/src/lib/utils/error-handling.ts | 70 ++------ packages/domain/common/errors.ts | 78 ++------- 6 files changed, 150 insertions(+), 248 deletions(-) create mode 100644 apps/bff/src/core/http/domain-http.exception.ts diff --git a/apps/bff/src/core/http/domain-http.exception.ts b/apps/bff/src/core/http/domain-http.exception.ts new file mode 100644 index 00000000..b379f126 --- /dev/null +++ b/apps/bff/src/core/http/domain-http.exception.ts @@ -0,0 +1,21 @@ +import { HttpException } from "@nestjs/common"; +import type { HttpStatus } from "@nestjs/common"; +import { ErrorMessages, type ErrorCodeType } from "@customer-portal/domain/common"; + +/** + * DomainHttpException + * + * Use this when you want to throw an HTTP error with an explicit, stable domain error code. + * The global exception filter will read `code` from the exception response and format the + * standard API error response for the client. + */ +export class DomainHttpException extends HttpException { + constructor( + public readonly code: ErrorCodeType, + status: HttpStatus, + message?: string + ) { + // `message` is optional; if omitted we fall back to the shared user-facing message. + super({ code, message: message ?? ErrorMessages[code] }, status); + } +} diff --git a/apps/bff/src/core/http/exception.filter.ts b/apps/bff/src/core/http/exception.filter.ts index 7d2b9444..ec622767 100644 --- a/apps/bff/src/core/http/exception.filter.ts +++ b/apps/bff/src/core/http/exception.filter.ts @@ -14,7 +14,6 @@ import { ErrorCode, ErrorMessages, ErrorMetadata, - matchErrorPattern, type ErrorCodeType, type ApiError, } from "@customer-portal/domain/common"; @@ -78,65 +77,92 @@ export class UnifiedExceptionFilter implements ExceptionFilter { } { let status = HttpStatus.INTERNAL_SERVER_ERROR; let originalMessage = "An unexpected error occurred"; + let explicitCode: ErrorCodeType | undefined; if (exception instanceof HttpException) { status = exception.getStatus(); - originalMessage = this.extractExceptionMessage(exception); + const extracted = this.extractExceptionDetails(exception); + originalMessage = extracted.message; + explicitCode = extracted.code; } else if (exception instanceof Error) { originalMessage = exception.message; } // Map to error code - const errorCode = this.mapToErrorCode(originalMessage, status); + const errorCode = explicitCode ?? this.mapStatusToErrorCode(status); return { status, errorCode, originalMessage }; } /** - * Extract message from HttpException + * Extract message (and optionally an explicit error code) from HttpException. */ - private extractExceptionMessage(exception: HttpException): string { + private extractExceptionDetails(exception: HttpException): { + message: string; + code?: ErrorCodeType; + } { const response = exception.getResponse(); if (typeof response === "string") { - return response; + return { message: response }; } if (typeof response === "object" && response !== null) { const responseObj = response as Record; + const code = this.extractExplicitCode(responseObj); // Handle NestJS validation errors (array of messages) if (Array.isArray(responseObj.message)) { const firstMessage = responseObj.message.find((m): m is string => typeof m === "string"); - if (firstMessage) return firstMessage; + if (firstMessage) return { message: firstMessage, code }; } // Handle standard message field if (typeof responseObj.message === "string") { - return responseObj.message; + return { message: responseObj.message, code }; } // Handle error field if (typeof responseObj.error === "string") { - return responseObj.error; + return { message: responseObj.error, code }; } + + return { message: exception.message, code }; } - return exception.message; + return { message: exception.message }; } /** - * Map error message and status to error code + * Extract explicit error code from HttpException response bodies. */ - private mapToErrorCode(message: string, status: number): ErrorCodeType { - // First, try pattern matching on the message - const patternCode = matchErrorPattern(message); - if (patternCode !== ErrorCode.UNKNOWN) { - return patternCode; + private extractExplicitCode(responseObj: Record): ErrorCodeType | undefined { + // 1) Preferred: { code: "AUTH_003" } + if (typeof responseObj.code === "string" && this.isKnownErrorCode(responseObj.code)) { + return responseObj.code as ErrorCodeType; } - // Fall back to status code mapping - // Cast status to HttpStatus to satisfy TypeScript enum comparison + // 2) Standard API error format: { error: { code: "AUTH_003" } } + const maybeError = responseObj.error; + if (maybeError && typeof maybeError === "object") { + const errorObj = maybeError as Record; + if (typeof errorObj.code === "string" && this.isKnownErrorCode(errorObj.code)) { + return errorObj.code as ErrorCodeType; + } + } + + return undefined; + } + + private isKnownErrorCode(value: string): boolean { + // ErrorMessages is keyed by ErrorCodeType + return Object.prototype.hasOwnProperty.call(ErrorMessages, value); + } + + /** + * Map status code to error code (no message parsing). + */ + private mapStatusToErrorCode(status: number): ErrorCodeType { switch (status as HttpStatus) { case HttpStatus.UNAUTHORIZED: return ErrorCode.SESSION_EXPIRED; diff --git a/apps/bff/src/integrations/whmcs/connection/services/whmcs-error-handler.service.ts b/apps/bff/src/integrations/whmcs/connection/services/whmcs-error-handler.service.ts index 0739a7d5..a4d8a19f 100644 --- a/apps/bff/src/integrations/whmcs/connection/services/whmcs-error-handler.service.ts +++ b/apps/bff/src/integrations/whmcs/connection/services/whmcs-error-handler.service.ts @@ -1,11 +1,11 @@ +import { Injectable, HttpStatus } from "@nestjs/common"; import { - Injectable, - NotFoundException, - BadRequestException, - UnauthorizedException, -} from "@nestjs/common"; + ErrorCode, + type ErrorCodeType, + type WhmcsErrorResponse, +} from "@customer-portal/domain/common"; +import { DomainHttpException } from "@bff/core/http/domain-http.exception.js"; import { getErrorMessage } from "@bff/core/utils/error.util.js"; -import type { WhmcsErrorResponse } from "@customer-portal/domain/common"; /** * Service for handling and normalizing WHMCS API errors @@ -24,69 +24,72 @@ export class WhmcsErrorHandlerService { const message = errorResponse.message; const errorCode = errorResponse.errorcode; - // Normalize common, expected error responses to domain exceptions - if (this.isNotFoundError(action, message)) { - throw this.createNotFoundException(action, message, params); - } - - if (this.isAuthenticationError(message, errorCode)) { - throw new UnauthorizedException(`WHMCS Authentication Error: ${message}`); - } - - if (this.isValidationError(message, errorCode)) { - throw new BadRequestException(`WHMCS Validation Error: ${message}`); - } - - // Generic WHMCS API error - throw new Error(`WHMCS API Error [${action}]: ${message} (${errorCode || "unknown"})`); + const mapped = this.mapProviderErrorToDomain(action, message, errorCode, params); + throw new DomainHttpException(mapped.code, mapped.status); } /** * Handle general request errors (network, timeout, etc.) */ - handleRequestError(error: unknown, action: string, _params: Record): never { - const message = getErrorMessage(error); - + handleRequestError(error: unknown, _action: string, _params: Record): never { if (this.isTimeoutError(error)) { - throw new Error(`WHMCS API timeout [${action}]: Request timed out`); + throw new DomainHttpException(ErrorCode.TIMEOUT, HttpStatus.GATEWAY_TIMEOUT); } if (this.isNetworkError(error)) { - throw new Error(`WHMCS API network error [${action}]: ${message}`); + throw new DomainHttpException(ErrorCode.NETWORK_ERROR, HttpStatus.BAD_GATEWAY); } - // Re-throw the original error if it's already a known exception type - if (this.isKnownException(error)) { - throw error; - } - - // Generic request error - throw new Error(`WHMCS API request failed [${action}]: ${message}`); + // If upstream already threw a DomainHttpException or HttpException with code, + // let the global exception filter handle it. + throw error; + } + + private mapProviderErrorToDomain( + action: string, + message: string, + providerErrorCode: string | undefined, + params: Record + ): { code: ErrorCodeType; status: HttpStatus } { + // 1) ValidateLogin: user credentials are wrong (expected) + if ( + action === "ValidateLogin" && + this.isValidateLoginInvalidCredentials(message, providerErrorCode) + ) { + return { code: ErrorCode.INVALID_CREDENTIALS, status: HttpStatus.UNAUTHORIZED }; + } + + // 2) Not-found style outcomes (expected for some reads) + if (this.isNotFoundError(action, message)) { + return { code: ErrorCode.NOT_FOUND, status: HttpStatus.NOT_FOUND }; + } + + // 3) WHMCS API key auth failures: external service/config problem (not end-user auth) + if (this.isAuthenticationError(message, providerErrorCode)) { + return { code: ErrorCode.EXTERNAL_SERVICE_ERROR, status: HttpStatus.SERVICE_UNAVAILABLE }; + } + + // 4) Validation failures: treat as bad request + if (this.isValidationError(message, providerErrorCode)) { + return { code: ErrorCode.VALIDATION_FAILED, status: HttpStatus.BAD_REQUEST }; + } + + // 5) Default: external service error + void params; // reserved for future mapping detail; keep signature stable + return { code: ErrorCode.EXTERNAL_SERVICE_ERROR, status: HttpStatus.BAD_GATEWAY }; } - /** - * Check if error indicates a not found condition - */ private isNotFoundError(action: string, message: string): boolean { const lowerMessage = message.toLowerCase(); - // Client not found errors - if (action === "GetClientsDetails" && lowerMessage.includes("client not found")) { - return true; - } - - // Invoice not found errors + if (action === "GetClientsDetails" && lowerMessage.includes("client not found")) return true; if ( (action === "GetInvoice" || action === "UpdateInvoice") && lowerMessage.includes("invoice not found") ) { return true; } - - // Product not found errors - if (action === "GetClientsProducts" && lowerMessage.includes("no products found")) { - return true; - } + if (action === "GetClientsProducts" && lowerMessage.includes("no products found")) return true; return false; } @@ -123,40 +126,19 @@ export class WhmcsErrorHandlerService { } /** - * Create appropriate NotFoundException based on action and params + * ValidateLogin returns user-facing "invalid credentials" strings (e.g. "Email or Password Invalid"). + * Treat these as authentication failures rather than external service errors. */ - private createNotFoundException( - action: string, - message: string, - params: Record - ): NotFoundException { - if (action === "GetClientsDetails") { - const emailParam = params["email"]; - if (typeof emailParam === "string") { - return new NotFoundException(`Client with email ${emailParam} not found`); - } + private isValidateLoginInvalidCredentials(message: string, errorCode?: string): boolean { + const lowerMessage = message.toLowerCase(); - const clientIdParam = params["clientid"]; - const identifier = - typeof clientIdParam === "string" || typeof clientIdParam === "number" - ? clientIdParam - : "unknown"; + // WHMCS commonly responds with: "Email or Password Invalid" + if (lowerMessage.includes("email or password invalid")) return true; + if (lowerMessage.includes("password invalid")) return true; + if (lowerMessage.includes("invalid login")) return true; + if (errorCode === "EMAIL_OR_PASSWORD_INVALID") return true; - return new NotFoundException(`Client with ID ${identifier} not found`); - } - - if (action === "GetInvoice" || action === "UpdateInvoice") { - const invoiceIdParam = params["invoiceid"]; - const identifier = - typeof invoiceIdParam === "string" || typeof invoiceIdParam === "number" - ? invoiceIdParam - : "unknown"; - - return new NotFoundException(`Invoice with ID ${identifier} not found`); - } - - // Generic not found error - return new NotFoundException(message); + return false; } /** @@ -185,33 +167,10 @@ export class WhmcsErrorHandlerService { ); } - /** - * Check if error is already a known NestJS exception - */ - private isKnownException(error: unknown): boolean { - return ( - error instanceof NotFoundException || - error instanceof BadRequestException || - error instanceof UnauthorizedException - ); - } - /** * Get user-friendly error message for client consumption */ getUserFriendlyMessage(error: unknown): string { - if (error instanceof NotFoundException) { - return "The requested resource was not found."; - } - - if (error instanceof BadRequestException) { - return "The request contains invalid data."; - } - - if (error instanceof UnauthorizedException) { - return "Authentication failed. Please check your credentials."; - } - const message = getErrorMessage(error).toLowerCase(); if (message.includes("timeout")) { diff --git a/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts b/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts index 3a752191..7bcecada 100644 --- a/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts +++ b/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts @@ -274,11 +274,13 @@ export class WhmcsHttpClientService { const errorMessage = this.toDisplayString(message ?? error, "Unknown WHMCS API error"); const errorCode = this.toDisplayString(errorcode, "unknown"); - this.logger.error(`WHMCS API returned error [${action}]`, { + // Many WHMCS "result=error" responses are expected business outcomes (e.g. invalid credentials). + // Log as warning (not error) to avoid spamming error logs; the unified exception filter will + // still emit the request-level log based on the mapped error code. + this.logger.warn(`WHMCS API returned error [${action}]`, { errorMessage, errorCode, params: this.sanitizeLogParams(params), - fullResponse: parsedResponse, }); // Return error response for the orchestrator to handle with proper exception types diff --git a/apps/portal/src/lib/utils/error-handling.ts b/apps/portal/src/lib/utils/error-handling.ts index cb580e16..f8be93bb 100644 --- a/apps/portal/src/lib/utils/error-handling.ts +++ b/apps/portal/src/lib/utils/error-handling.ts @@ -10,7 +10,6 @@ import { ErrorCode, ErrorMessages, ErrorMetadata, - matchErrorPattern, type ErrorCodeType, } from "@customer-portal/domain/common"; @@ -46,13 +45,11 @@ export function parseError(error: unknown): ParsedError { // Handle string errors if (typeof error === "string") { - const code = matchErrorPattern(error); - const metadata = ErrorMetadata[code]; return { - code, + code: ErrorCode.UNKNOWN, message: error, - shouldLogout: metadata.shouldLogout, - shouldRetry: metadata.shouldRetry, + shouldLogout: false, + shouldRetry: true, }; } @@ -77,11 +74,7 @@ function parseApiError(error: ClientApiError): ParsedError { const bodyObj = body as Record; // Check for standard { success: false, error: { code, message } } format - if ( - bodyObj.success === false && - bodyObj.error && - typeof bodyObj.error === "object" - ) { + if (bodyObj.success === false && bodyObj.error && typeof bodyObj.error === "object") { const errorObj = bodyObj.error as Record; const code = typeof errorObj.code === "string" ? errorObj.code : undefined; const message = typeof errorObj.message === "string" ? errorObj.message : undefined; @@ -97,18 +90,8 @@ function parseApiError(error: ClientApiError): ParsedError { } } - // Try extracting message from body - const extractedMessage = extractMessageFromBody(body); - if (extractedMessage) { - const code = matchErrorPattern(extractedMessage); - const metadata = ErrorMetadata[code]; - return { - code, - message: extractedMessage, - shouldLogout: metadata.shouldLogout, - shouldRetry: metadata.shouldRetry, - }; - } + // No message-based code inference. If the response doesn't include a structured error code, + // we fall back to status-based mapping below. } // Fall back to status code mapping @@ -146,46 +129,14 @@ function parseNativeError(error: Error): ParsedError { }; } - // Try pattern matching on error message - const code = matchErrorPattern(error.message); - const metadata = ErrorMetadata[code]; return { - code, - message: code === ErrorCode.UNKNOWN ? error.message : ErrorMessages[code], - shouldLogout: metadata.shouldLogout, - shouldRetry: metadata.shouldRetry, + code: ErrorCode.UNKNOWN, + message: error.message || ErrorMessages[ErrorCode.UNKNOWN], + shouldLogout: false, + shouldRetry: true, }; } -/** - * Extract error message from response body - */ -function extractMessageFromBody(body: unknown): string | null { - if (!body || typeof body !== "object") { - return null; - } - - const bodyObj = body as Record; - - // Check nested error.message (standard format) - if (bodyObj.error && typeof bodyObj.error === "object") { - const errorObj = bodyObj.error as Record; - if (typeof errorObj.message === "string") { - return errorObj.message; - } - } - - // Check top-level message - if (typeof bodyObj.message === "string") { - return bodyObj.message; - } - - return null; -} - -/** - * Map HTTP status code to error code - */ function mapStatusToErrorCode(status?: number): ErrorCodeType { if (!status) return ErrorCode.UNKNOWN; @@ -249,6 +200,5 @@ export { ErrorCode, ErrorMessages, ErrorMetadata, - matchErrorPattern, type ErrorCodeType, } from "@customer-portal/domain/common"; diff --git a/packages/domain/common/errors.ts b/packages/domain/common/errors.ts index e0628f49..5b54f19b 100644 --- a/packages/domain/common/errors.ts +++ b/packages/domain/common/errors.ts @@ -109,16 +109,19 @@ export const ErrorMessages: Record = { [ErrorCode.CUSTOMER_NOT_FOUND]: "Customer account not found. Please contact support.", [ErrorCode.ORDER_ALREADY_PROCESSED]: "This order has already been processed.", [ErrorCode.INSUFFICIENT_BALANCE]: "Insufficient account balance.", - [ErrorCode.SERVICE_UNAVAILABLE]: "This service is temporarily unavailable. Please try again later.", + [ErrorCode.SERVICE_UNAVAILABLE]: + "This service is temporarily unavailable. Please try again later.", // System [ErrorCode.INTERNAL_ERROR]: "An unexpected error occurred. Please try again later.", - [ErrorCode.EXTERNAL_SERVICE_ERROR]: "An external service is temporarily unavailable. Please try again later.", + [ErrorCode.EXTERNAL_SERVICE_ERROR]: + "An external service is temporarily unavailable. Please try again later.", [ErrorCode.DATABASE_ERROR]: "A system error occurred. Please try again later.", [ErrorCode.CONFIGURATION_ERROR]: "A system configuration error occurred. Please contact support.", // Network - [ErrorCode.NETWORK_ERROR]: "Unable to connect to the server. Please check your internet connection.", + [ErrorCode.NETWORK_ERROR]: + "Unable to connect to the server. Please check your internet connection.", [ErrorCode.TIMEOUT]: "The request timed out. Please try again.", [ErrorCode.RATE_LIMITED]: "Too many requests. Please wait a moment and try again.", @@ -162,7 +165,9 @@ export const ErrorMetadata: Record = { severity: "low", shouldLogout: true, shouldRetry: false, - logLevel: "info", + // Session expiry is an expected flow (browser tabs, refresh loops, etc.) and can be extremely noisy. + // Keep it available for debugging but avoid spamming production logs at info level. + logLevel: "debug", }, [ErrorCode.TOKEN_INVALID]: { category: "authentication", @@ -378,68 +383,8 @@ export function canRetryError(code: string): boolean { return getErrorMetadata(code).shouldRetry; } -// ============================================================================ -// Pattern Matching for Error Classification -// ============================================================================ - -interface ErrorPattern { - pattern: RegExp; - code: ErrorCodeType; -} - -/** - * Patterns to match error messages to error codes. - * Used when explicit error codes are not available. - */ -export const ErrorPatterns: ErrorPattern[] = [ - // Authentication patterns - { pattern: /invalid.*credentials?|wrong.*password|invalid.*password/i, code: ErrorCode.INVALID_CREDENTIALS }, - { pattern: /account.*locked|locked.*account|too.*many.*attempts/i, code: ErrorCode.ACCOUNT_LOCKED }, - { pattern: /session.*expired|expired.*session/i, code: ErrorCode.SESSION_EXPIRED }, - { pattern: /token.*expired|expired.*token/i, code: ErrorCode.SESSION_EXPIRED }, - { pattern: /token.*revoked|revoked.*token/i, code: ErrorCode.TOKEN_REVOKED }, - { pattern: /invalid.*token|token.*invalid/i, code: ErrorCode.TOKEN_INVALID }, - { pattern: /refresh.*token.*invalid|invalid.*refresh/i, code: ErrorCode.REFRESH_TOKEN_INVALID }, - - // Authorization patterns - { pattern: /admin.*required|requires?.*admin/i, code: ErrorCode.ADMIN_REQUIRED }, - { pattern: /forbidden|not.*authorized|unauthorized/i, code: ErrorCode.FORBIDDEN }, - { pattern: /access.*denied|permission.*denied/i, code: ErrorCode.RESOURCE_ACCESS_DENIED }, - - // Business patterns - { pattern: /already.*exists|email.*exists|account.*exists/i, code: ErrorCode.ACCOUNT_EXISTS }, - { pattern: /already.*linked/i, code: ErrorCode.ACCOUNT_ALREADY_LINKED }, - { pattern: /customer.*not.*found|account.*not.*found/i, code: ErrorCode.CUSTOMER_NOT_FOUND }, - { pattern: /already.*processed/i, code: ErrorCode.ORDER_ALREADY_PROCESSED }, - { pattern: /insufficient.*balance/i, code: ErrorCode.INSUFFICIENT_BALANCE }, - - // System patterns - { pattern: /database|sql|postgres|prisma|connection.*refused/i, code: ErrorCode.DATABASE_ERROR }, - { pattern: /whmcs|salesforce|external.*service/i, code: ErrorCode.EXTERNAL_SERVICE_ERROR }, - { pattern: /configuration.*error|missing.*config/i, code: ErrorCode.CONFIGURATION_ERROR }, - - // Network patterns - { pattern: /network.*error|fetch.*failed|econnrefused/i, code: ErrorCode.NETWORK_ERROR }, - { pattern: /timeout|timed?\s*out/i, code: ErrorCode.TIMEOUT }, - { pattern: /too.*many.*requests|rate.*limit/i, code: ErrorCode.RATE_LIMITED }, - - // Validation patterns (lower priority - checked last) - { pattern: /not.*found/i, code: ErrorCode.NOT_FOUND }, - { pattern: /validation.*failed|invalid/i, code: ErrorCode.VALIDATION_FAILED }, - { pattern: /required|missing/i, code: ErrorCode.REQUIRED_FIELD_MISSING }, -]; - -/** - * Match an error message to an error code using patterns - */ -export function matchErrorPattern(message: string): ErrorCodeType { - for (const { pattern, code } of ErrorPatterns) { - if (pattern.test(message)) { - return code; - } - } - return ErrorCode.UNKNOWN; -} +// NOTE: We intentionally do NOT support matching error messages to codes. +// Error codes must be explicit and stable (returned from the API or thrown by server code). // ============================================================================ // Zod Schema for Error Response @@ -476,4 +421,3 @@ export function createErrorResponse( }, }; } -