From 690c4683292474c318a97cae067f3ca646d6684b Mon Sep 17 00:00:00 2001 From: jmgasper Date: Tue, 22 Sep 2026 17:00:47 +1000 Subject: [PATCH] Reduce Salesforce report calls and log quota errors --- SALES.md | 13 +-- src/reports/sales/sales-reports.dto.ts | 2 +- src/reports/sales/sales-reports.service.ts | 6 +- src/reports/sales/sales-reports.spec.ts | 2 +- .../sales/salesforce-reports.client.spec.ts | 28 ++++++ .../sales/salesforce-reports.client.ts | 86 +++++++++++++++++-- 6 files changed, 119 insertions(+), 18 deletions(-) diff --git a/SALES.md b/SALES.md index 4055cd1..7969589 100644 --- a/SALES.md +++ b/SALES.md @@ -48,7 +48,7 @@ Both endpoints accept the same query parameters: | `sortBy`, `sortOrder` | Column ID and `asc`/`desc`; numeric and ISO date values sort before pagination | | `dateColumn` | Column ID of a `date`/`datetime` column, such as Created Date or Close Date | | `dateFrom`, `dateTo` | Inclusive `YYYY-MM-DD` bounds; either or both, and both require `dateColumn` | -| `refresh` | `true` to refresh, subject to the five-second minimum interval; default `false` | +| `refresh` | `true` to refresh, subject to the one-minute minimum interval; default `false` | Response fields: `reportId`, `reportName`, `columns[{id,label,dataType}]`, `rows[{id,cells:[{label,value,currencyCode?}]}]`, `allData`, `sourceRowCount`, @@ -129,16 +129,19 @@ and [report execution contract](https://developer.salesforce.com/docs/analytics/ ## Freshness, failures and extension One in-memory snapshot per service instance lasts 60 seconds. Concurrent reads -share an in-flight request; manual refresh has a five-second cooldown. No report -data is persisted. A failed refresh returns an error, with a five-second retry +share an in-flight request; manual refresh uses the same one-minute interval. No report +data is persisted. A failed refresh returns an error, with a one-minute retry cooldown, and never changes the last successful timestamp. The UI refreshes visible pages every minute and on return to a visible tab; hidden tabs do not poll. It displays stale-data status when a refresh fails. OAuth and report requests time out after 15 seconds per attempt. Network failures, HTTP 429 and 5xx retry up to three attempts with bounded backoff; -401 report responses renew OAuth once. Errors and logs omit tokens and upstream -response bodies. Validation returns 400, missing configuration 503, and upstream +401 report responses renew OAuth once. A Salesforce synchronous report quota error +pauses report calls from that instance for five minutes. Warnings include the HTTP +status, bounded Salesforce error code/message, configured report ID, and Salesforce +request ID when present; tokens and full upstream response bodies are omitted. +Validation returns 400, missing configuration 503, and upstream failures 502. Authorization returns 401/403 before Salesforce is contacted. Future reports can reuse `SalesforceReportsClient.runReport(reportId)` and the diff --git a/src/reports/sales/sales-reports.dto.ts b/src/reports/sales/sales-reports.dto.ts index 556d292..1f0be96 100644 --- a/src/reports/sales/sales-reports.dto.ts +++ b/src/reports/sales/sales-reports.dto.ts @@ -135,7 +135,7 @@ export class SalesReportQueryDto { @ApiPropertyOptional({ default: false, - description: "Refresh Salesforce data (minimum five-second interval).", + description: "Refresh Salesforce data (minimum one-minute interval).", }) @IsOptional() @Transform(({ value }: { value: unknown }) => diff --git a/src/reports/sales/sales-reports.service.ts b/src/reports/sales/sales-reports.service.ts index e6028ab..39bb3de 100644 --- a/src/reports/sales/sales-reports.service.ts +++ b/src/reports/sales/sales-reports.service.ts @@ -21,7 +21,7 @@ import { } from "./salesforce-reports.client"; const CACHE_MS = 60000; -const REFRESH_COOLDOWN_MS = 5000; +const REFRESH_COOLDOWN_MS = CACHE_MS; /** Column types that can carry a pipeline or revenue amount worth totalling. */ const AMOUNT_TYPES = ["currency", "double"]; /** Column types that a date range can be applied to. */ @@ -388,9 +388,9 @@ export class SalesReportsService { /** * Loads or reuses the current snapshot. Failed refreshes never relabel stale data as fresh. - * @param refresh Whether to bypass the regular TTL, subject to a five-second cooldown. + * @param refresh Whether to request a refresh, subject to the one-minute cache interval. * @returns A report fetched within the cache interval. - * @throws The sanitized client/normalization error; failures are throttled for five seconds. + * @throws The sanitized client/normalization error; failures are throttled for one minute. */ private async getSnapshot(refresh: boolean): Promise { if (this.loading) return this.loading; diff --git a/src/reports/sales/sales-reports.spec.ts b/src/reports/sales/sales-reports.spec.ts index f0f1298..1493009 100644 --- a/src/reports/sales/sales-reports.spec.ts +++ b/src/reports/sales/sales-reports.spec.ts @@ -244,7 +244,7 @@ describe("SalesReportsService", () => { Object.assign(new SalesReportQueryDto(), { refresh: true }), ); expect(runReport).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(5001); + jest.advanceTimersByTime(60001); await service.getReport( Object.assign(new SalesReportQueryDto(), { refresh: true }), ); diff --git a/src/reports/sales/salesforce-reports.client.spec.ts b/src/reports/sales/salesforce-reports.client.spec.ts index 9194167..f51866d 100644 --- a/src/reports/sales/salesforce-reports.client.spec.ts +++ b/src/reports/sales/salesforce-reports.client.spec.ts @@ -76,6 +76,34 @@ describe("SalesforceReportsClient", () => { ); expect(request).toHaveBeenCalledTimes(5); }); + it("logs the Salesforce quota reason and pauses further report calls", async () => { + const warn = jest.spyOn(client["logger"], "warn").mockImplementation(); + request + .mockResolvedValueOnce(oauth()) + .mockResolvedValueOnce( + Response.json( + [ + { + errorCode: "FORBIDDEN", + message: + "You can't run more than 500 reports synchronously every 60 minutes. Try again later. test-secret", + }, + ], + { status: 403 }, + ), + ); + await expect(client.runReport(reportId)).rejects.toBeInstanceOf( + BadGatewayException, + ); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("FORBIDDEN: You can't run more than 500 reports"), + ); + expect(warn.mock.calls[0][0]).not.toContain("test-secret"); + await expect(client.runReport(reportId)).rejects.toThrow( + "quota is temporarily exhausted", + ); + expect(request).toHaveBeenCalledTimes(2); + }); it("bounds network retries and sanitizes errors", async () => { request.mockRejectedValue(new Error("secret network details")); await expect(client.runReport(reportId)).rejects.toBeInstanceOf( diff --git a/src/reports/sales/salesforce-reports.client.ts b/src/reports/sales/salesforce-reports.client.ts index 99654e5..f9ce9d1 100644 --- a/src/reports/sales/salesforce-reports.client.ts +++ b/src/reports/sales/salesforce-reports.client.ts @@ -51,6 +51,7 @@ export class SalesforceReportsClient { private readonly logger = new Logger(SalesforceReportsClient.name); private session?: SalesforceSession; private authenticating?: Promise; + private quotaRetryAt = 0; /** @param config Server environment configuration. Creates a lazy client; does not authenticate or throw. */ constructor(private readonly config: ConfigService) {} @@ -130,6 +131,74 @@ export class SalesforceReportsClient { ); } + /** + * Logs bounded Salesforce error details without logging the full response or credentials. + * @param response Failed OAuth or report response. + * @param operation Safe operation name and, for reports, the configured report ID. + * @returns True when Salesforce says the synchronous report quota is exhausted. + * @throws Does not throw for missing, malformed, or unreadable error bodies. + */ + private async logRejection( + response: Response, + operation: string, + ): Promise { + let details = ""; + let quotaExceeded = false; + try { + const body: unknown = await response.json(); + const errors = Array.isArray(body) ? body : [body]; + details = errors + .slice(0, 3) + .map((error: unknown) => { + if (!error || typeof error !== "object") return ""; + const item = error as Record; + const code = + typeof item.errorCode === "string" + ? item.errorCode + : typeof item.error === "string" + ? item.error + : ""; + const message = + typeof item.message === "string" + ? item.message + : typeof item.error_description === "string" + ? item.error_description + : ""; + if ( + response.status === 403 && + /more than 500 reports synchronously every 60 minutes/i.test( + message, + ) + ) + quotaExceeded = true; + const safeCode = code.replace(/[^a-zA-Z0-9_]/g, "").slice(0, 64); + let safeMessage = message + .replace(/[\r\n\t\x00-\x1f\x7f]/g, " ") + .replace(/Bearer\s+\S+/gi, "Bearer [redacted]") + .slice(0, 240); + for (const secret of [ + this.config.get("SALESFORCE_API_CONSUMER_SECRET"), + this.session?.access_token, + ]) { + if (secret) safeMessage = safeMessage.split(secret).join("[redacted]"); + } + return [safeCode, safeMessage].filter(Boolean).join(": "); + }) + .filter(Boolean) + .join("; "); + } catch { + // JSON parsing may already have consumed or locked the response stream. + } + const requestId = response.headers + .get("sforce-request-id") + ?.replace(/[^a-zA-Z0-9-]/g, "") + .slice(0, 80); + this.logger.warn( + `Salesforce ${operation} rejected (HTTP ${response.status})${details ? `: ${details}` : ""}${requestId ? `; requestId=${requestId}` : ""}.`, + ); + return quotaExceeded; + } + /** * Obtains a client-credentials session, coalescing concurrent token requests. * @returns A trusted instance URL and an access token kept only in server memory. @@ -164,10 +233,7 @@ export class SalesforceReportsClient { }), }); if (!response.ok) { - await response.body?.cancel(); - this.logger.warn( - `Salesforce authentication rejected (HTTP ${response.status}).`, - ); + await this.logRejection(response, "authentication"); throw new BadGatewayException( "Salesforce authentication failed. Contact your administrator.", ); @@ -220,6 +286,11 @@ export class SalesforceReportsClient { "Salesforce API version is not configured correctly.", ); } + if (Date.now() < this.quotaRetryAt) { + throw new BadGatewayException( + "Salesforce report quota is temporarily exhausted. Please try again later.", + ); + } for (let attempt = 0; attempt < 2; attempt++) { const session = await this.authenticate(); const response = await this.request( @@ -237,10 +308,9 @@ export class SalesforceReportsClient { continue; } if (!response.ok) { - await response.body?.cancel(); - this.logger.warn( - `Salesforce report request rejected (HTTP ${response.status}).`, - ); + if (await this.logRejection(response, `report ${reportId}`)) { + this.quotaRetryAt = Date.now() + 5 * 60 * 1000; + } throw new BadGatewayException( "Salesforce report could not be loaded. Please try again.", );