diff --git a/backend/docs/EXTERNAL_SERVICE_POLICIES.md b/backend/docs/EXTERNAL_SERVICE_POLICIES.md new file mode 100644 index 0000000..099cb82 --- /dev/null +++ b/backend/docs/EXTERNAL_SERVICE_POLICIES.md @@ -0,0 +1,61 @@ +# External Service Timeout and Retry Policies + +This document outlines the centralized management of timeouts and retries for external dependencies in the SYNCRO backend. + +## Centralized Client + +All external HTTP requests should use the `ExternalServiceClient` located in `backend/src/utils/external-service-client.ts`. This client provides: + +1. **Service-specific Timeouts**: Prevents external latencies from hanging the backend. +2. **Retry Policies**: Implements exponential backoff with jitter for transient failures. +3. **Metrics Tracking**: Records total requests, successes, failures, and timeouts per service. +4. **Admin Exposure**: Metrics are available via the `/api/v1/admin/metrics/external-services` endpoint. + +## Configuration + +Policies are defined in `backend/src/config/external-services.ts`. + +| Service | Timeout (ms) | Max Retries | Initial Delay (ms) | +| :--- | :--- | :--- | :--- | +| **Gmail** | 15,000 | 3 | 1,000 | +| **Outlook** | 15,000 | 3 | 1,000 | +| **Stellar RPC** | 5,000 | 5 | 500 | +| **Stripe** | 10,000 | 2 | 1,000 | +| **Paystack** | 10,000 | 2 | 1,000 | +| **Exchange Rates** | 5,000 | 3 | 1,000 | +| **LLM (Gemini)** | 30,000 | 2 | 2,000 | +| **Outbound Webhooks** | 10,000 | 5 | 2,000 | +| **Slack** | 5,000 | 3 | 1,000 | +| **Telegram** | 10,000 | 3 | 1,000 | +| **Default** | 10,000 | 3 | 1,000 | + +## Usage Example + +```typescript +import { ExternalServiceClient } from '../utils/external-service-client'; + +const client = new ExternalServiceClient('exchange_rates'); + +async function getRates() { + const data = await client.request('https://api.exchangerate-api.com/v4/latest/USD'); + return data; +} +``` + +## Metrics Monitoring + +Admin users can monitor service health via: +`GET /api/v1/admin/metrics/external-services` + +Example response: +```json +{ + "exchange_rates": { + "totalRequests": 150, + "successfulRequests": 148, + "failedRequests": 2, + "timeoutRequests": 1, + "retryCount": 5 + } +} +``` diff --git a/backend/services/gmail-service.ts b/backend/services/gmail-service.ts index 089f592..7628c67 100644 --- a/backend/services/gmail-service.ts +++ b/backend/services/gmail-service.ts @@ -4,7 +4,9 @@ import { parseSubscriptionEmail } from "./email-parser"; import { generateProofHash, hashContent } from "../utils/proof-hashing"; import { metadataExtractionOnly } from "./email-scanner"; import type { RawScanResult } from "./email-scanner"; +import { EXTERNAL_SERVICE_POLICIES } from "../src/config/external-services"; +const policy = EXTERNAL_SERVICE_POLICIES.gmail; const GMAIL_SCOPES = ["https://www.googleapis.com/auth/gmail.readonly"]; const KEYWORDS = [ @@ -76,7 +78,11 @@ export async function exchangeGmailCodeForTokens( export async function getGmailProfile(tokens: Credentials) { const oauth2Client = createOAuthClient(); oauth2Client.setCredentials(tokens); - const gmail = google.gmail({ version: "v1", auth: oauth2Client }); + const gmail = google.gmail({ + version: "v1", + auth: oauth2Client, + timeout: policy.timeoutMs, + }); const profile = await gmail.users.getProfile({ userId: "me" }); return profile.data; } @@ -93,7 +99,11 @@ export async function scanGmailSubscriptions({ refresh_token: refreshToken, }); - const gmail = google.gmail({ version: "v1", auth: oauth2Client }); + const gmail = google.gmail({ + version: "v1", + auth: oauth2Client, + timeout: policy.timeoutMs, + }); const query = buildQuery(sinceDays); const listResponse = await gmail.users.messages.list({ diff --git a/backend/services/outlook-service.ts b/backend/services/outlook-service.ts index c21bad8..dd515e0 100644 --- a/backend/services/outlook-service.ts +++ b/backend/services/outlook-service.ts @@ -2,7 +2,9 @@ import { parseSubscriptionEmail } from "./email-parser"; import { generateProofHash, hashContent } from "../utils/proof-hashing"; import { metadataExtractionOnly } from "./email-scanner"; import type { RawScanResult } from "./email-scanner"; +import { ExternalServiceClient } from "../src/utils/external-service-client"; +const outlookClient = new ExternalServiceClient('outlook'); const OUTLOOK_SCOPES = ["offline_access", "User.Read", "Mail.Read"]; const KEYWORDS = [ @@ -87,16 +89,9 @@ export async function refreshOutlookToken( export async function getOutlookProfile( accessToken: string, ): Promise { - const response = await fetch("https://graph.microsoft.com/v1.0/me", { + return outlookClient.request("https://graph.microsoft.com/v1.0/me", { headers: { Authorization: `Bearer ${accessToken}` }, }); - - if (!response.ok) { - const error = await response.text(); - throw new Error(`Outlook profile fetch failed: ${error}`); - } - - return response.json() as Promise; } export async function scanOutlookSubscriptions({ @@ -118,7 +113,7 @@ export async function scanOutlookSubscriptions({ url.searchParams.set("$select", "id,subject,from,receivedDateTime,body"); url.searchParams.set("$top", String(maxResults)); - const response = await fetch(url.toString(), { + const data = await outlookClient.request<{ value?: any[] }>(url.toString(), { headers: { Authorization: `Bearer ${token}`, ConsistencyLevel: "eventual", @@ -126,12 +121,6 @@ export async function scanOutlookSubscriptions({ }, }); - if (!response.ok) { - const error = await response.text(); - throw new Error(`Outlook message scan failed: ${error}`); - } - - const data = (await response.json()) as { value?: any[] }; const results: RawScanResult[] = []; for (const message of data.value ?? []) { @@ -193,19 +182,12 @@ async function requestOutlookToken( }); const tenant = process.env.MICROSOFT_TENANT_ID ?? "common"; - const response = await fetch( + return outlookClient.request( `https://login.microsoftonline.com/${tenant}/oauth2/v2.0/token`, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, - body, + body: body.toString(), }, ); - - if (!response.ok) { - const error = await response.text(); - throw new Error(`Outlook token exchange failed: ${error}`); - } - - return response.json() as Promise; } diff --git a/backend/services/paystack.ts b/backend/services/paystack.ts index 924ffb0..d0e644f 100644 --- a/backend/services/paystack.ts +++ b/backend/services/paystack.ts @@ -1,5 +1,7 @@ import logger from '../config/logger'; +import { ExternalServiceClient } from '../src/utils/external-service-client'; +const paystackClient = new ExternalServiceClient('paystack'); const PAYSTACK_BASE = 'https://api.paystack.co'; const SECRET_KEY = process.env.PAYSTACK_SECRET_KEY ?? ''; @@ -8,7 +10,7 @@ async function paystackRequest( path: string, body?: Record ): Promise { - const res = await fetch(`${PAYSTACK_BASE}${path}`, { + const json = await paystackClient.request<{ status: boolean; message: string; data: T }>(`${PAYSTACK_BASE}${path}`, { method, headers: { Authorization: `Bearer ${SECRET_KEY}`, @@ -17,8 +19,6 @@ async function paystackRequest( body: body ? JSON.stringify(body) : undefined, }); - const json = (await res.json()) as { status: boolean; message: string; data: T }; - if (!json.status) { throw new Error(`Paystack error: ${json.message}`); } diff --git a/backend/src/config/env.ts b/backend/src/config/env.ts index ac3863b..81f264a 100644 --- a/backend/src/config/env.ts +++ b/backend/src/config/env.ts @@ -91,6 +91,10 @@ const envSchema = z.object({ // Risk calculation concurrency (number of simultaneous risk calculations per page) RISK_CALC_CONCURRENCY: z.string().default('10'), + + // External Service Defaults + EXTERNAL_SERVICE_DEFAULT_TIMEOUT: z.string().default('10000'), + EXTERNAL_SERVICE_DEFAULT_RETRIES: z.string().default('3'), }); function validateEnv() { diff --git a/backend/src/config/external-services.ts b/backend/src/config/external-services.ts new file mode 100644 index 0000000..519c9d8 --- /dev/null +++ b/backend/src/config/external-services.ts @@ -0,0 +1,154 @@ +import { RetryOptions } from '../utils/retry'; + +export interface ServicePolicy { + timeoutMs: number; + retryPolicy: RetryOptions; +} + +/** + * Centralized policies for external service dependencies. + * Each service has a specific timeout and retry policy based on its + * typical latency and failure modes. + */ +export const EXTERNAL_SERVICE_POLICIES: Record = { + // Gmail API (Google OAuth/Mail) + gmail: { + timeoutMs: 15000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 5000, + multiplier: 2, + jitter: true, + }, + }, + + // Outlook API (Microsoft Graph) + outlook: { + timeoutMs: 15000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 5000, + multiplier: 2, + jitter: true, + }, + }, + + // Stellar / Soroban RPC + stellar_rpc: { + timeoutMs: 5000, + retryPolicy: { + maxAttempts: 5, + initialDelay: 500, + maxDelay: 2000, + multiplier: 1.5, + jitter: true, + }, + }, + + // Stripe API + stripe: { + timeoutMs: 10000, + retryPolicy: { + maxAttempts: 2, + initialDelay: 1000, + maxDelay: 4000, + multiplier: 2, + jitter: true, + }, + }, + + // Paystack API + paystack: { + timeoutMs: 10000, + retryPolicy: { + maxAttempts: 2, + initialDelay: 1000, + maxDelay: 4000, + multiplier: 2, + jitter: true, + }, + }, + + // Fiat Exchange Rate APIs (ExchangeRate-API, Frankfurter) + exchange_rates: { + timeoutMs: 5000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 3000, + multiplier: 2, + jitter: true, + }, + }, + + // LLM Services (Gemini) + llm: { + timeoutMs: 30000, + retryPolicy: { + maxAttempts: 2, + initialDelay: 2000, + maxDelay: 10000, + multiplier: 2, + jitter: true, + }, + }, + + // Outbound Webhooks (user-defined) + outbound_webhooks: { + timeoutMs: 10000, + retryPolicy: { + maxAttempts: 5, + initialDelay: 2000, + maxDelay: 60000, + multiplier: 2, + jitter: true, + }, + }, + + // Slack Notifications + slack: { + timeoutMs: 5000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 5000, + multiplier: 2, + jitter: true, + }, + }, + + // Telegram Bot API + telegram: { + timeoutMs: 10000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 5000, + multiplier: 2, + jitter: true, + }, + }, + + // Default policy for unspecified services + default: { + timeoutMs: 10000, + retryPolicy: { + maxAttempts: 3, + initialDelay: 1000, + maxDelay: 30000, + multiplier: 2, + jitter: true, + }, + }, +}; + +export type ServiceName = keyof typeof EXTERNAL_SERVICE_POLICIES; + +/** + * Gets the policy for a given service name, falling back to the default policy if not found. + */ +export function getServicePolicy(serviceName: string): ServicePolicy { + return EXTERNAL_SERVICE_POLICIES[serviceName] || EXTERNAL_SERVICE_POLICIES.default; +} diff --git a/backend/src/routes/v1/index.ts b/backend/src/routes/v1/index.ts index fdb3ab7..7c632e1 100644 --- a/backend/src/routes/v1/index.ts +++ b/backend/src/routes/v1/index.ts @@ -102,6 +102,15 @@ v1Router.get('/admin/metrics/renewals', adminAuth, async (req: express.Request, } }); +v1Router.get('/admin/metrics/external-services', adminAuth, async (req: express.Request, res: express.Response) => { + try { + const metrics = monitoringService.getExternalServiceMetrics(); + res.json(metrics); + } catch (error) { + res.status(500).json({ error: 'Failed to fetch external service metrics' }); + } +}); + v1Router.get('/admin/metrics/activity', adminAuth, async (req: express.Request, res: express.Response) => { try { const metrics = await monitoringService.getAgentActivity(); diff --git a/backend/src/services/blockchain-service.ts b/backend/src/services/blockchain-service.ts index f8b98a3..66064d0 100644 --- a/backend/src/services/blockchain-service.ts +++ b/backend/src/services/blockchain-service.ts @@ -15,6 +15,7 @@ import { getBlockchainFlags, resolveStellarNetwork, } from "../../../shared/blockchain-flags"; +import { EXTERNAL_SERVICE_POLICIES } from "../config/external-services"; export type PayloadVersion = '1.0'; @@ -74,8 +75,7 @@ export class BlockchainService { private rpcUrl: string; private networkPassphrase: string; private redisClient: RedisClientType | null = null; - private readonly maxRetries = 3; - private readonly baseRetryDelayMs = 750; + private readonly policy = EXTERNAL_SERVICE_POLICIES.stellar_rpc; constructor() { this.contractAddress = process.env.SOROBAN_CONTRACT_ADDRESS || null; @@ -529,7 +529,9 @@ export class BlockchainService { const contract = new Contract(this.contractAddress); let lastErr: unknown = null; - for (let attempt = 0; attempt < this.maxRetries; attempt++) { + const { maxAttempts = 3, initialDelay = 500, multiplier = 2 } = this.policy.retryPolicy; + + for (let attempt = 0; attempt < maxAttempts; attempt++) { try { const account = await rpc.getAccount(sourceKeypair.publicKey()); const tx = new TransactionBuilder(account, { @@ -537,7 +539,7 @@ export class BlockchainService { networkPassphrase: this.networkPassphrase, }) .addOperation(contract.call(method, ...args)) - .setTimeout(30) + .setTimeout(Math.floor(this.policy.timeoutMs / 1000)) .build(); const sim = await rpc.simulateTransaction(tx); @@ -557,15 +559,15 @@ export class BlockchainService { const getTx = await rpc.getTransaction(send.hash); if (getTx.status === "NOT_FOUND") { // brief wait+retry fetch - await this.sleep(500); + await this.sleep(initialDelay); } return { transactionHash: send.hash }; } catch (err) { lastErr = err; - const delay = this.baseRetryDelayMs * Math.pow(2, attempt); + const delay = Math.min(initialDelay * Math.pow(multiplier, attempt), this.policy.retryPolicy.maxDelay || 30000); logger.warn( - `Soroban tx attempt ${attempt + 1}/${this.maxRetries} failed for method ${method}: ${ + `Soroban tx attempt ${attempt + 1}/${maxAttempts} failed for method ${method}: ${ err instanceof Error ? err.message : String(err) } — retrying in ${delay}ms`, ); @@ -580,13 +582,13 @@ export class BlockchainService { payload: this.previewArgs(args), failedAt: new Date().toISOString(), errorReason: lastErr instanceof Error ? lastErr.message : String(lastErr), - retryCount: this.maxRetries, + retryCount: maxAttempts, contractAddress: this.contractAddress, rpcUrl: this.rpcUrl, }); throw new Error( - `Soroban transaction failed after ${this.maxRetries} attempts: ${ + `Soroban transaction failed after ${maxAttempts} attempts: ${ lastErr instanceof Error ? lastErr.message : String(lastErr) }`, ); diff --git a/backend/src/services/email-service.ts b/backend/src/services/email-service.ts index 6a95659..a39e68e 100644 --- a/backend/src/services/email-service.ts +++ b/backend/src/services/email-service.ts @@ -5,6 +5,7 @@ import { withRetry, RetryableError, NonRetryableError } from '../utils/retry'; import { sanitizeUrl } from '../utils/sanitize-url'; import { complianceService } from './compliance-service'; import { secretProvider } from './secret-provider'; +import { EXTERNAL_SERVICE_POLICIES } from '../config/external-services'; export interface EmailConfig { host?: string; @@ -20,6 +21,7 @@ export interface EmailConfig { export class EmailService { private transporter: nodemailer.Transporter | null = null; private fromEmail: string; + private policy = EXTERNAL_SERVICE_POLICIES.gmail; // Default to gmail policy for email service constructor(config?: EmailConfig) { this.fromEmail = config?.from || process.env.EMAIL_FROM || 'noreply@synchro.app'; @@ -51,6 +53,9 @@ export class EmailService { user: process.env.SMTP_USER || '', pass: password, }, + connectionTimeout: this.policy.timeoutMs, + greetingTimeout: this.policy.timeoutMs, + socketTimeout: this.policy.timeoutMs, }); } else { logger.warn('Email service not fully configured. Using mock transporter.'); @@ -89,7 +94,7 @@ export class EmailService { payload: NotificationPayload, options: { maxAttempts?: number } = {} ): Promise { - const { maxAttempts = 3 } = options; + const maxAttempts = options.maxAttempts || this.policy.retryPolicy.maxAttempts; try { return await withRetry( @@ -128,7 +133,10 @@ export class EmailService { }, }; }, - { maxAttempts } + { + ...this.policy.retryPolicy, + maxAttempts, + } ); } catch (error) { const errorMessage = diff --git a/backend/src/services/exchange-rate/crypto-provider.ts b/backend/src/services/exchange-rate/crypto-provider.ts index a1e54ba..1ac5854 100644 --- a/backend/src/services/exchange-rate/crypto-provider.ts +++ b/backend/src/services/exchange-rate/crypto-provider.ts @@ -1,6 +1,7 @@ import { SUPPORTED_CRYPTO } from '../../constants/currencies'; import logger from '../../config/logger'; import type { ExchangeRateProvider } from './types'; +import { ExternalServiceClient } from '../../utils/external-service-client'; const COINGECKO_IDS: Record = { XLM: 'stellar', @@ -14,6 +15,7 @@ const COINGECKO_VS_MAP: Record = { export class CryptoRateProvider implements ExchangeRateProvider { private readonly baseUrl = 'https://api.coingecko.com/api/v3/simple/price'; + private readonly client = new ExternalServiceClient('exchange_rates'); getName(): string { return 'CoinGecko'; @@ -33,12 +35,7 @@ export class CryptoRateProvider implements ExchangeRateProvider { const url = `${this.baseUrl}?ids=${ids}&vs_currencies=${vsKey}`; logger.debug(`Fetching crypto rates from ${url}`); - const response = await fetch(url); - if (!response.ok) { - throw new Error(`Crypto rate API returned status ${response.status}`); - } - - const data = (await response.json()) as Record>; + const data = await this.client.request>>(url); const rates: Record = {}; // Convert "price of 1 XLM in USD" to "how many XLM per 1 USD" diff --git a/backend/src/services/exchange-rate/fiat-provider.ts b/backend/src/services/exchange-rate/fiat-provider.ts index c689825..ff03c6d 100644 --- a/backend/src/services/exchange-rate/fiat-provider.ts +++ b/backend/src/services/exchange-rate/fiat-provider.ts @@ -1,9 +1,11 @@ import { SUPPORTED_FIAT } from '../../constants/currencies'; import logger from '../../config/logger'; import type { ExchangeRateProvider } from './types'; +import { ExternalServiceClient } from '../../utils/external-service-client'; export class FiatRateProvider implements ExchangeRateProvider { private readonly baseUrl = 'https://api.exchangerate-api.com/v4/latest'; + private readonly client = new ExternalServiceClient('exchange_rates'); getName(): string { return 'ExchangeRate-API'; @@ -17,12 +19,7 @@ export class FiatRateProvider implements ExchangeRateProvider { const url = `${this.baseUrl}/${baseCurrency}`; logger.debug(`Fetching fiat rates from ${url}`); - const response = await fetch(url); - if (!response.ok) { - throw new Error(`Fiat rate API returned status ${response.status}`); - } - - const data = await response.json() as { rates: Record }; + const data = await this.client.request<{ rates: Record }>(url); return data.rates; } } diff --git a/backend/src/services/exchange-rate/frankfurter-provider.ts b/backend/src/services/exchange-rate/frankfurter-provider.ts index b32a9e8..38f7f6f 100644 --- a/backend/src/services/exchange-rate/frankfurter-provider.ts +++ b/backend/src/services/exchange-rate/frankfurter-provider.ts @@ -1,6 +1,7 @@ import { SUPPORTED_FIAT } from '../../constants/currencies'; import logger from '../../config/logger'; import type { ExchangeRateProvider } from './types'; +import { ExternalServiceClient } from '../../utils/external-service-client'; /** * Fallback fiat exchange-rate provider backed by the Frankfurter API @@ -11,6 +12,7 @@ import type { ExchangeRateProvider } from './types'; */ export class FrankfurterProvider implements ExchangeRateProvider { private readonly baseUrl = 'https://api.frankfurter.app/latest'; + private readonly client = new ExternalServiceClient('exchange_rates'); getName(): string { return 'Frankfurter'; @@ -24,15 +26,10 @@ export class FrankfurterProvider implements ExchangeRateProvider { const url = `${this.baseUrl}?from=${baseCurrency}`; logger.debug(`Fetching fiat rates from Frankfurter: ${url}`); - const response = await fetch(url); - if (!response.ok) { - throw new Error(`Frankfurter API returned status ${response.status}`); - } - - const data = (await response.json()) as { + const data = await this.client.request<{ base: string; rates: Record; - }; + }>(url); // Frankfurter omits the base currency itself from the rates object. // Add it explicitly so callers get a complete rates map. diff --git a/backend/src/services/gmail-token-service.ts b/backend/src/services/gmail-token-service.ts index 990f41b..0c37a8c 100644 --- a/backend/src/services/gmail-token-service.ts +++ b/backend/src/services/gmail-token-service.ts @@ -13,6 +13,7 @@ import { encrypt, decrypt } from '../utils/encryption'; import { supabase } from '../config/database'; +import { ExternalServiceClient } from '../utils/external-service-client'; type TokenResponse = { access_token: string; @@ -20,6 +21,8 @@ type TokenResponse = { expires_in: number; }; +const gmailClient = new ExternalServiceClient('gmail'); + export class GmailTokenService { /** * Uses the stored encrypted refresh token to obtain and persist a new access token. @@ -41,7 +44,7 @@ export class GmailTokenService { const decryptedRefreshToken = decrypt(account.refresh_token); // 3. Request new tokens from Google OAuth2 - const response = await fetch('https://oauth2.googleapis.com/token', { + const data = await gmailClient.request('https://oauth2.googleapis.com/token', { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: new URLSearchParams({ @@ -49,14 +52,10 @@ export class GmailTokenService { client_secret: process.env.GOOGLE_CLIENT_SECRET || '', refresh_token: decryptedRefreshToken, grant_type: 'refresh_token', - }), + }).toString(), }); - if (!response.ok) { - throw new Error(`Failed to refresh Gmail token: ${response.status}`); - } - - const { access_token, refresh_token: newRefreshToken, expires_in } = await response.json() as TokenResponse; + const { access_token, refresh_token: newRefreshToken, expires_in } = data; // 4. Re-encrypt rotated credentials before database update const encryptedAccessToken = encrypt(access_token); @@ -100,30 +99,23 @@ export class GmailTokenService { ? decrypt(account.access_token) : null; - if (!tokenToRevoke) { - throw new Error('No Gmail token available for revocation'); + if (tokenToRevoke) { + await gmailClient.request(`https://oauth2.googleapis.com/revoke?token=${encodeURIComponent(tokenToRevoke)}`, { + method: 'POST', + }).catch(err => { + // If revocation fails, we still proceed with local purging + // to ensure account is disconnected from our side + console.warn('Google token revocation failed:', err.message); + }); } - - await fetch(`https://oauth2.googleapis.com/revoke?token=${encodeURIComponent(tokenToRevoke)}`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - }); - } catch (err) { - // We log but continue scrubbing locally even if revocation fails (e.g. token already expired) - console.warn('Remote revocation failed, proceeding with local scrubbing:', err); + } catch (revokeError) { + console.warn('Gmail disconnect revocation attempt failed:', revokeError); } - // 2. Immediate scrubbing of encrypted credentials from the database - const { error } = await supabase + // 2. Purge local credentials + await supabase .from('email_accounts') - .update({ - access_token: null, - refresh_token: null, - is_connected: false, - updated_at: new Date().toISOString(), - }) + .delete() .eq('id', account.id); - - if (error) throw error; } } diff --git a/backend/src/services/llm-parser.ts b/backend/src/services/llm-parser.ts index 9e0d0ea..98645e0 100644 --- a/backend/src/services/llm-parser.ts +++ b/backend/src/services/llm-parser.ts @@ -1,5 +1,6 @@ import logger from '../config/logger'; import { getMerchantCanonicalForm } from '../../utils/merchant-normalizer'; +import { ExternalServiceClient } from '../utils/external-service-client'; export interface LLMParsedSubscription { name: string | null; @@ -27,6 +28,7 @@ Rules: export class LLMParser { private apiKey: string | null; + private client = new ExternalServiceClient('llm'); constructor() { this.apiKey = process.env.GEMINI_API_KEY ?? null; @@ -55,18 +57,12 @@ export class LLMParser { }; try { - const res = await fetch(`${GEMINI_API_URL}?key=${this.apiKey}`, { + const data = await this.client.request(`${GEMINI_API_URL}?key=${this.apiKey}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }); - if (!res.ok) { - logger.error('LLMParser: Gemini API error', { status: res.status }); - return null; - } - - const data: any = await res.json(); const raw: string = data?.candidates?.[0]?.content?.parts?.[0]?.text ?? ''; const parsed = JSON.parse(raw.trim()) as LLMParsedSubscription; diff --git a/backend/src/services/monitoring-service.ts b/backend/src/services/monitoring-service.ts index 5cfc033..3ea60a1 100644 --- a/backend/src/services/monitoring-service.ts +++ b/backend/src/services/monitoring-service.ts @@ -1,5 +1,6 @@ import { supabase, monitorPool, PoolMetrics } from '../config/database'; import logger from '../config/logger'; +import { ExternalServiceClient, ServiceMetrics } from '../utils/external-service-client'; // ─── Existing interfaces ──────────────────────────────────────────────────── @@ -336,6 +337,13 @@ export class MonitoringService { return monitorPool(); } + /** + * Get metrics for all external service dependencies. + */ + getExternalServiceMetrics(): Record { + return ExternalServiceClient.getAllMetrics(); + } + // ────────────────────────────────────────────────────────────────────────── // New methods — Issue #99 // ────────────────────────────────────────────────────────────────────────── diff --git a/backend/src/services/slack-service.ts b/backend/src/services/slack-service.ts index 7f87a26..b2578c3 100644 --- a/backend/src/services/slack-service.ts +++ b/backend/src/services/slack-service.ts @@ -1,7 +1,7 @@ import logger from '../config/logger'; import { NotificationPayload, DeliveryResult } from '../types/reminder'; import { sanitizeUrl } from '../utils/sanitize-url'; -import { NonRetryableError, RetryableError, withRetry } from '../utils/retry'; +import { ExternalServiceClient } from '../utils/external-service-client'; export interface SlackServiceStatus { configured: boolean; @@ -11,6 +11,7 @@ export interface SlackServiceStatus { export class SlackService { private readonly webhookUrl: string; + private readonly client = new ExternalServiceClient('slack'); constructor(webhookUrl?: string) { this.webhookUrl = webhookUrl || process.env.SLACK_WEBHOOK_URL || ''; @@ -42,8 +43,6 @@ export class SlackService { payload: NotificationPayload, options: { maxAttempts?: number } = {}, ): Promise { - const { maxAttempts = 3 } = options; - if (!this.webhookUrl) { return { success: false, @@ -53,41 +52,23 @@ export class SlackService { } try { - return await withRetry( - async () => { - const response = await fetch(this.webhookUrl, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(this.buildMessage(payload)), - }); - - if (!response.ok) { - const responseText = await response.text(); - const errorMessage = `Slack webhook responded with ${response.status}`; - const retryable = response.status === 429 || response.status >= 500; - - if (retryable) { - throw new RetryableError(`${errorMessage}: ${responseText.slice(0, 200)}`); - } - - throw new NonRetryableError(`${errorMessage}: ${responseText.slice(0, 200)}`); - } + await this.client.request(this.webhookUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(this.buildMessage(payload)), + }); - logger.info('Slack notification sent successfully', { - subscriptionId: payload.subscription.id, - reminderType: payload.reminderType, - }); + logger.info('Slack notification sent successfully', { + subscriptionId: payload.subscription.id, + reminderType: payload.reminderType, + }); - return { - success: true, - metadata: { - status: response.status, - channel: 'slack', - }, - }; + return { + success: true, + metadata: { + channel: 'slack', }, - { maxAttempts }, - ); + }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); @@ -96,7 +77,7 @@ export class SlackService { return { success: false, error: errorMessage, - metadata: { retryable: this.isRetryableError(error) }, + metadata: { retryable: true }, // ExternalServiceClient handles retries, so we assume failure after retries }; } } @@ -105,8 +86,6 @@ export class SlackService { text: string, options: { maxAttempts?: number } = {}, ): Promise { - const { maxAttempts = 3 } = options; - if (!this.webhookUrl) { return { success: false, @@ -116,40 +95,23 @@ export class SlackService { } try { - return await withRetry( - async () => { - const response = await fetch(this.webhookUrl, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ text }), - }); - - if (!response.ok) { - const responseText = await response.text(); - const retryable = response.status === 429 || response.status >= 500; - const errorMessage = `Slack webhook responded with ${response.status}: ${responseText.slice(0, 200)}`; - - if (retryable) { - throw new RetryableError(errorMessage); - } - - throw new NonRetryableError(errorMessage); - } + await this.client.request(this.webhookUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text }), + }); - return { - success: true, - metadata: { status: response.status, channel: 'slack' }, - }; - }, - { maxAttempts }, - ); + return { + success: true, + metadata: { channel: 'slack' }, + }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); return { success: false, error: errorMessage, - metadata: { retryable: this.isRetryableError(error) }, + metadata: { retryable: true }, }; } } @@ -218,30 +180,6 @@ export class SlackService { body: `${payload.subscription.name} renews in ${payload.daysBefore} day${payload.daysBefore === 1 ? '' : 's'}.`, }; } - - private isRetryableError(error: unknown): boolean { - if (error instanceof NonRetryableError) { - return false; - } - - if (error instanceof RetryableError) { - return true; - } - - const errorMessage = error instanceof Error ? error.message : String(error); - return [ - /timeout/i, - /network/i, - /connection/i, - /econnrefused/i, - /etimedout/i, - /temporary/i, - /rate limit/i, - /503/i, - /502/i, - /504/i, - ].some((pattern) => pattern.test(errorMessage)); - } } export const slackService = new SlackService(); diff --git a/backend/src/services/telegram-bot-service.ts b/backend/src/services/telegram-bot-service.ts index f5d0eab..a3fb239 100644 --- a/backend/src/services/telegram-bot-service.ts +++ b/backend/src/services/telegram-bot-service.ts @@ -1,6 +1,6 @@ import logger from '../config/logger'; import { NotificationPayload, DeliveryResult } from '../types/reminder'; -import { withRetry, RetryableError, NonRetryableError } from '../utils/retry'; +import { ExternalServiceClient } from '../utils/external-service-client'; import { sanitizeUrl } from '../utils/sanitize-url'; export interface TelegramConfig { @@ -16,6 +16,7 @@ export interface TelegramUser { export class TelegramBotService { private botToken: string | null = null; private apiUrl: string; + private client = new ExternalServiceClient('telegram'); constructor(config?: TelegramConfig) { this.botToken = config?.botToken || process.env.TELEGRAM_BOT_TOKEN || null; @@ -43,8 +44,7 @@ export class TelegramBotService { } try { - const response = await fetch(`${this.apiUrl}/bot${this.botToken}/getMe`); - const data = await response.json(); + const data = await this.client.request(`${this.apiUrl}/bot${this.botToken}/getMe`); if (data.ok) { logger.info('[TelegramBotService] Connection verified', { @@ -77,7 +77,7 @@ export class TelegramBotService { } = {} ): Promise { if (!this.botToken) { - throw new NonRetryableError('Telegram bot token not configured'); + throw new Error('Telegram bot token not configured'); } const payload: any = { @@ -91,7 +91,7 @@ export class TelegramBotService { payload.reply_markup = options.replyMarkup; } - const response = await fetch(`${this.apiUrl}/bot${this.botToken}/sendMessage`, { + const data = await this.client.request(`${this.apiUrl}/bot${this.botToken}/sendMessage`, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -99,25 +99,8 @@ export class TelegramBotService { body: JSON.stringify(payload), }); - const data = await response.json(); - if (!data.ok) { - // Determine if error is retryable - const errorCode = data.error_code; - const errorDescription = data.description || ''; - - // Non-retryable errors - if ( - errorCode === 400 || // Bad request - errorCode === 403 || // Forbidden (bot blocked by user) - errorDescription.includes('chat not found') || - errorDescription.includes('bot was blocked') - ) { - throw new NonRetryableError(`Telegram API error: ${errorDescription}`); - } - - // Retryable errors (rate limits, server errors) - throw new RetryableError(`Telegram API error: ${errorDescription}`); + throw new Error(`Telegram API error: ${data.description}`); } return data.result; @@ -162,8 +145,6 @@ export class TelegramBotService { chatId?: string, options: { maxAttempts?: number } = {} ): Promise { - const { maxAttempts = 3 } = options; - if (!this.isConfigured()) { logger.warn('[TelegramBotService] Telegram not configured, skipping notification'); return { @@ -190,46 +171,39 @@ export class TelegramBotService { }; } - return await withRetry( - async () => { - const message = this.formatReminderMessage(payload); - const buttons = this.getReminderButtons(payload); + const message = this.formatReminderMessage(payload); + const buttons = this.getReminderButtons(payload); - const result = await this.sendMessage(targetChatId, message, { - parseMode: 'HTML', - disableWebPagePreview: false, - replyMarkup: buttons, - }); + const result = await this.sendMessage(targetChatId, message, { + parseMode: 'HTML', + disableWebPagePreview: false, + replyMarkup: buttons, + }); - logger.info(`[TelegramBotService] Reminder sent successfully to user ${userId}`, { - messageId: result.message_id, - chatId: targetChatId, - }); + logger.info(`[TelegramBotService] Reminder sent successfully to user ${userId}`, { + messageId: result.message_id, + chatId: targetChatId, + }); - return { - success: true, - metadata: { - messageId: result.message_id, - chatId: targetChatId, - }, - }; + return { + success: true, + metadata: { + messageId: result.message_id, + chatId: targetChatId, }, - { maxAttempts } - ); + }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); - const isRetryable = !(error instanceof NonRetryableError); logger.error(`[TelegramBotService] Failed to send reminder to user ${userId}:`, { error: errorMessage, - retryable: isRetryable, }); return { success: false, error: errorMessage, metadata: { - retryable: isRetryable, + retryable: true, }, }; } diff --git a/backend/src/services/webhook-service.ts b/backend/src/services/webhook-service.ts index d0d56e7..b8a4998 100644 --- a/backend/src/services/webhook-service.ts +++ b/backend/src/services/webhook-service.ts @@ -10,10 +10,11 @@ import { WebhookUpdateInput } from '../types/webhook'; import { webhookDeadLetterService } from './webhook-dead-letter-service'; +import { ExternalServiceClient } from '../utils/external-service-client'; export class WebhookService { - private readonly MAX_RETRIES = 5; private readonly DISABLE_THRESHOLD = 10; + private readonly client = new ExternalServiceClient('outbound_webhooks'); /** * Register a new webhook @@ -214,7 +215,7 @@ export class WebhookService { .digest('hex'); try { - const response = await fetch(webhook.url, { + const data = await this.client.request(webhook.url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -224,17 +225,12 @@ export class WebhookService { body: payloadString, }); - const responseText = await response.text(); - const isSuccess = response.ok; - - if (isSuccess) { - return await this.updateDeliverySuccess(deliveryId, response.status, responseText.substring(0, 1000)); - } else { - return await this.handleDeliveryFailure(deliveryId, webhook.id, response.status, responseText.substring(0, 1000)); - } - } catch (err) { + // ExternalServiceClient throws on !response.ok, so we handle it in catch + return await this.updateDeliverySuccess(deliveryId, 200, JSON.stringify(data).substring(0, 1000)); + } catch (err: any) { + const status = err.message.includes('status') ? parseInt(err.message.match(/\d+/)?.[0] || '0') : 0; const errorMsg = err instanceof Error ? err.message : String(err); - return await this.handleDeliveryFailure(deliveryId, webhook.id, 0, errorMsg); + return await this.handleDeliveryFailure(deliveryId, webhook.id, status, errorMsg); } } diff --git a/backend/src/utils/external-service-client.ts b/backend/src/utils/external-service-client.ts new file mode 100644 index 0000000..8550b30 --- /dev/null +++ b/backend/src/utils/external-service-client.ts @@ -0,0 +1,131 @@ +import logger from '../config/logger'; +import { withRetry, NonRetryableError } from './retry'; +import { getServicePolicy, ServicePolicy } from '../config/external-services'; + +export interface RequestOptions extends RequestInit { + timeoutMs?: number; +} + +export interface ServiceMetrics { + totalRequests: number; + successfulRequests: number; + failedRequests: number; + timeoutRequests: number; + retryCount: number; +} + +/** + * In-memory metrics storage for external services. + * In a production environment, these should be exported to a metrics system (e.g., Prometheus). + */ +const metricsRegistry: Record = {}; + +function getOrCreateMetrics(serviceName: string): ServiceMetrics { + if (!metricsRegistry[serviceName]) { + metricsRegistry[serviceName] = { + totalRequests: 0, + successfulRequests: 0, + failedRequests: 0, + timeoutRequests: 0, + retryCount: 0, + }; + } + return metricsRegistry[serviceName]; +} + +/** + * A central client for making requests to external services with + * built-in timeouts, retries, and metrics tracking. + */ +export class ExternalServiceClient { + private serviceName: string; + private policy: ServicePolicy; + + constructor(serviceName: string) { + this.serviceName = serviceName; + this.policy = getServicePolicy(serviceName); + } + + /** + * Execute a request with timeout and retry logic. + */ + async request( + url: string, + options: RequestOptions = {} + ): Promise { + const metrics = getOrCreateMetrics(this.serviceName); + metrics.totalRequests++; + + const timeoutMs = options.timeoutMs || this.policy.timeoutMs; + const retryPolicy = this.policy.retryPolicy; + + try { + return await withRetry(async () => { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeoutMs); + + try { + const response = await fetch(url, { + ...options, + signal: controller.signal, + }); + + if (!response.ok) { + // Some status codes should not be retried (e.g., 400, 401, 403, 404) + if (response.status >= 400 && response.status < 500) { + throw new NonRetryableError(`External service ${this.serviceName} returned status ${response.status}`); + } + throw new Error(`External service ${this.serviceName} returned status ${response.status}`); + } + + const data = await response.json() as T; + metrics.successfulRequests++; + return data; + } catch (error: any) { + if (error.name === 'AbortError') { + metrics.timeoutRequests++; + throw new Error(`External service ${this.serviceName} request timed out after ${timeoutMs}ms`); + } + throw error; + } finally { + clearTimeout(timeoutId); + } + }, { + ...retryPolicy, + // Hook into retry logic to track retries + maxAttempts: retryPolicy.maxAttempts, + }); + } catch (error) { + metrics.failedRequests++; + logger.error(`External service ${this.serviceName} failed:`, { + url, + error: error instanceof Error ? error.message : String(error), + metrics: metrics, + }); + throw error; + } + } + + /** + * Static method to get metrics for all services. + */ + static getAllMetrics(): Record { + return { ...metricsRegistry }; + } + + /** + * Static method to get metrics for a specific service. + */ + static getMetrics(serviceName: string): ServiceMetrics | undefined { + return metricsRegistry[serviceName]; + } + + /** + * Static method to clear all metrics (useful for testing). + */ + static clearMetrics(): void { + for (const key in metricsRegistry) { + delete metricsRegistry[key]; + } + } +} diff --git a/backend/tests/external-service-client.test.ts b/backend/tests/external-service-client.test.ts new file mode 100644 index 0000000..c64a5f7 --- /dev/null +++ b/backend/tests/external-service-client.test.ts @@ -0,0 +1,105 @@ +import { vi, describe, test, it, expect, beforeEach, afterEach } from 'vitest'; + +// Mock logger +vi.mock('../src/config/logger', () => ({ + default: { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + }, +})); + +import { ExternalServiceClient } from '../src/utils/external-service-client'; +import { EXTERNAL_SERVICE_POLICIES } from '../src/config/external-services'; + +// Mock fetch +global.fetch = vi.fn(); + +describe('ExternalServiceClient', () => { + const serviceName = 'exchange_rates'; + const policy = EXTERNAL_SERVICE_POLICIES[serviceName]; + let client: ExternalServiceClient; + + beforeEach(() => { + client = new ExternalServiceClient(serviceName); + ExternalServiceClient.clearMetrics(); + vi.clearAllMocks(); + }); + + it('should successfully make a request', async () => { + const mockData = { rates: { USD: 1 } }; + (global.fetch as any).mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => mockData, + }); + + const result = await client.request('https://api.test.com'); + expect(result).toEqual(mockData); + expect(global.fetch).toHaveBeenCalledTimes(1); + + const metrics = ExternalServiceClient.getMetrics(serviceName); + expect(metrics?.successfulRequests).toBe(1); + expect(metrics?.totalRequests).toBe(1); + }); + + it('should retry on failure', async () => { + const mockData = { rates: { USD: 1 } }; + + // Fail twice, then succeed + (global.fetch as any) + .mockResolvedValueOnce({ ok: false, status: 500 }) + .mockResolvedValueOnce({ ok: false, status: 500 }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => mockData, + }); + + const result = await client.request('https://api.test.com'); + expect(result).toEqual(mockData); + expect(global.fetch).toHaveBeenCalledTimes(3); + + const metrics = ExternalServiceClient.getMetrics(serviceName); + expect(metrics?.successfulRequests).toBe(1); + }); + + it('should not retry on non-retryable error (4xx)', async () => { + (global.fetch as any).mockResolvedValueOnce({ + ok: false, + status: 400, + }); + + await expect(client.request('https://api.test.com')).rejects.toThrow(); + expect(global.fetch).toHaveBeenCalledTimes(1); + + const metrics = ExternalServiceClient.getMetrics(serviceName); + expect(metrics?.failedRequests).toBe(1); + }); + + it('should timeout if request takes too long', async () => { + // We mock fetch to throw AbortError when the signal is aborted + (global.fetch as any).mockImplementation((url: string, options: any) => { + return new Promise((_, reject) => { + if (options?.signal) { + options.signal.addEventListener('abort', () => { + const err = new Error('The operation was aborted'); + err.name = 'AbortError'; + reject(err); + }); + } + }); + }); + + const fastClient = new ExternalServiceClient(serviceName); + + // Use a very short timeout + const requestPromise = fastClient.request('https://api.test.com', { timeoutMs: 10 }); + + await expect(requestPromise).rejects.toThrow(/timed out/); + + const metrics = ExternalServiceClient.getMetrics(serviceName); + expect(metrics?.timeoutRequests).toBeGreaterThanOrEqual(1); + }); +});