diff --git a/packages/backend/src/apps/m365-excel/common/interceptors/request-error-handler.ts b/packages/backend/src/apps/m365-excel/common/interceptors/request-error-handler.ts new file mode 100644 index 000000000..6b8d8be9b --- /dev/null +++ b/packages/backend/src/apps/m365-excel/common/interceptors/request-error-handler.ts @@ -0,0 +1,26 @@ +import type { IApp } from '@plumber/types' + +import logger from '@/helpers/logger' + +const http429Handler: IApp['requestErrorHandler'] = async function ($, error) { + if (error.response.status !== 429) { + return + } + + // A 429 response is considered a SEV-2+ incident for some tenants; log it + // explicitly so that we can easily trigger incident creation from DD. + logger.error('Received HTTP 429 from MS Graph', { + event: 'm365-http-429', + tenant: $.auth?.data?.tenantKey as string, + baseUrl: error.response.config.baseURL, + url: error.response.config.url, + flowId: $.flow?.id, + stepId: $.step?.id, + executionId: $.execution?.id, + }) + + // We don't want to retry 429s from M365, so convert it into a non-HttpError. + throw new Error('Rate limited by Microsoft Graph.') +} + +export default http429Handler diff --git a/packages/backend/src/apps/m365-excel/index.ts b/packages/backend/src/apps/m365-excel/index.ts index d73d50da3..65e1198bf 100644 --- a/packages/backend/src/apps/m365-excel/index.ts +++ b/packages/backend/src/apps/m365-excel/index.ts @@ -1,6 +1,7 @@ import type { IApp } from '@plumber/types' import beforeRequest from './common/interceptors/before-request' +import requestErrorHandler from './common/interceptors/request-error-handler' import actions from './actions' import auth from './auth' import dynamicData from './dynamic-data' @@ -16,6 +17,7 @@ const app: IApp = { auth, actions, beforeRequest, + requestErrorHandler, dynamicData, } diff --git a/packages/backend/src/helpers/global-variable.ts b/packages/backend/src/helpers/global-variable.ts index a8d5115b1..20ec18164 100644 --- a/packages/backend/src/helpers/global-variable.ts +++ b/packages/backend/src/helpers/global-variable.ts @@ -120,7 +120,8 @@ const globalVariable = async ( $.http = createHttpClient({ $, baseURL: app.apiBaseUrl, - beforeRequest: app.beforeRequest, + beforeRequest: app.beforeRequest ?? [], + requestErrorHandler: app.requestErrorHandler ?? null, }) if (flow) { diff --git a/packages/backend/src/helpers/http-client/index.ts b/packages/backend/src/helpers/http-client/index.ts index bcad0cb97..36ac2915d 100644 --- a/packages/backend/src/helpers/http-client/index.ts +++ b/packages/backend/src/helpers/http-client/index.ts @@ -25,7 +25,8 @@ const removeBaseUrlForAbsoluteUrls = ( export default function createHttpClient({ $, baseURL, - beforeRequest = [], + beforeRequest, + requestErrorHandler, }: IHttpClientParams) { const instance = axios.create({ baseURL, @@ -90,6 +91,26 @@ export default function createHttpClient({ throw new HttpError(error) }, ) + // We use a separate interceptor for requestErrorHandler to allow the above + // HttpError inteceptor to throw early. + instance.interceptors.response.use( + (response) => response, + async (error) => { + if (!requestErrorHandler) { + throw error + } + + // Passthrough other errors... although other errors should really only be + // RetriableError. + if (!(error instanceof HttpError)) { + throw error + } + + await requestErrorHandler($, error) + + throw error + }, + ) return instance } diff --git a/packages/types/index.d.ts b/packages/types/index.d.ts index 1d0e4f183..157410e93 100644 --- a/packages/types/index.d.ts +++ b/packages/types/index.d.ts @@ -1,3 +1,4 @@ +import HttpError from '@/errors/http' import type { AxiosInstance, AxiosRequestConfig, @@ -273,15 +274,32 @@ export interface IApp { actions?: IAction[] connections?: IConnection[] description?: string -} -export type TBeforeRequest = { - ( - $: IGlobalVariable, - requestConfig: InternalAxiosRequestConfig, - ): Promise + /** + * A callback that is invoked if there's an error for any HTTP request this + * app makes using $.http. + * + * This is useful to perform per-request monitoring or error transformations + * (e.g logging on specific HTTP response codes or converting 429s to a + * non-HttpError to prevent automated retries). + * + * We support this because if an app needs custom error monitoring for _all_ + * requests, it allows us to stop having to remember to surround all our code + * with try / catch. + */ + requestErrorHandler?: TRequestErrorHandler } +export type TBeforeRequest = ( + $: IGlobalVariable, + requestConfig: InternalAxiosRequestConfig, +) => Promise + +export type TRequestErrorHandler = ( + $: IGlobalVariable, + error: HttpError, +) => Promise + export interface DynamicDataOutput { data: { name: string @@ -473,7 +491,8 @@ export interface ISubstep { export type IHttpClientParams = { $: IGlobalVariable baseURL?: string - beforeRequest?: TBeforeRequest[] + beforeRequest: TBeforeRequest[] + requestErrorHandler: TRequestErrorHandler } export type IGlobalVariable = {