From 78ba02946b413672c75a4dbd85e85c2ec819012d Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Tue, 3 Feb 2026 16:18:33 +0545 Subject: [PATCH 1/2] fix(OUT-3045): retry mechanisms for dropbox rate limit or server error - [x] trigger retry when hitting dropbox rate limit or server error - [x] logic to handle dropbox rate limit case and retry after specified time in response header --- src/errors/BaseServerError.ts | 7 ++-- src/lib/dropbox/DropboxClient.ts | 24 +++++-------- src/lib/withRetry.ts | 61 ++++++++++++++++++-------------- src/utils/sleep.ts | 1 + src/utils/withErrorHandler.ts | 6 ++++ 5 files changed, 53 insertions(+), 46 deletions(-) create mode 100644 src/utils/sleep.ts diff --git a/src/errors/BaseServerError.ts b/src/errors/BaseServerError.ts index 8cd2810..fd6460a 100644 --- a/src/errors/BaseServerError.ts +++ b/src/errors/BaseServerError.ts @@ -1,6 +1,5 @@ export interface StatusableError extends Error { status: number - retryAfter?: number } /** @@ -11,18 +10,16 @@ export class BaseServerError extends Error { constructor( message: string, public readonly statusCode: number, - retryAfter?: number, ) { super(message) this.name = 'BaseServerError' - this.retryAfter = retryAfter } } export const baseServerErrorFactory = (name: string, message: string, statusCode: number) => { return class extends BaseServerError { - constructor(messageOverride?: string, retryAfter?: number) { - super(messageOverride || message, statusCode, retryAfter) + constructor(messageOverride?: string) { + super(messageOverride || message, statusCode) this.name = name } } diff --git a/src/lib/dropbox/DropboxClient.ts b/src/lib/dropbox/DropboxClient.ts index 97dee12..38fd90e 100644 --- a/src/lib/dropbox/DropboxClient.ts +++ b/src/lib/dropbox/DropboxClient.ts @@ -1,11 +1,10 @@ -import { Dropbox, type DropboxAuth, type files } from 'dropbox' +import { Dropbox, type DropboxAuth, DropboxResponseError, type files } from 'dropbox' import httpStatus from 'http-status' import { camelKeys } from 'js-convert-case' import fetch from 'node-fetch' import env from '@/config/server.env' import { MAX_FETCH_DBX_RESOURCES } from '@/constants/limits' import { DropboxClientType, type DropboxClientTypeValue } from '@/db/constants' -import type { StatusableError } from '@/errors/BaseServerError' import { DropboxAuthClient } from '@/lib/dropbox/DropboxAuthClient' import { type DropboxFileMetadata, DropboxFileMetadataSchema } from '@/lib/dropbox/type' @@ -63,22 +62,12 @@ export class DropboxClient { otherOptions?: Record, method: 'GET' | 'POST' | 'PUT' | 'DELETE' = 'POST', ) { - const response = await fetch(url, { + return await fetch(url, { method, headers, body, ...otherOptions, }) - - if (!response.ok) { - const retryAfter = response.headers.get('Retry-After') - const error = new Error(`Request failed with status ${response.status}`) as StatusableError - error.status = response.status - error.retryAfter = retryAfter ? parseInt(retryAfter, 10) : undefined - throw error - } - - return response } async _getAllFilesFolders( @@ -126,7 +115,9 @@ export class DropboxClient { } const response = await this.manualFetch(`${env.DROPBOX_API_URL}${urlPath}`, headers) if (response.status !== httpStatus.OK) - throw new Error('DropboxClient#downloadFile. Failed to download file') + throw new DropboxResponseError(response.status, response.headers, { + error_summary: 'DropboxClient#downloadFile. Failed to download file', // following the dropbox response error convention with snake case + }) return response.body } @@ -156,8 +147,11 @@ export class DropboxClient { 'Content-Type': 'application/octet-stream', } const response = await this.manualFetch(`${env.DROPBOX_API_URL}${urlPath}`, headers, body) + if (response.status !== httpStatus.OK) - throw new Error('DropboxClient#uploadFile. Failed to upload file') + throw new DropboxResponseError(response.status, response.headers, { + error_summary: 'DropboxClient#uploadFile. Failed to upload file', // following the dropbox response error convention with snake case + }) return DropboxFileMetadataSchema.parse(camelKeys(await response.json())) } diff --git a/src/lib/withRetry.ts b/src/lib/withRetry.ts index 6315c62..9373f32 100644 --- a/src/lib/withRetry.ts +++ b/src/lib/withRetry.ts @@ -2,8 +2,10 @@ import Sentry from '@sentry/nextjs' import { DropboxResponseError } from 'dropbox' +import httpStatus from 'http-status' import pRetry from 'p-retry' import type { StatusableError } from '@/errors/BaseServerError' +import { sleep } from '@/utils/sleep' export const withRetry = async ( fn: (...args: Args) => Promise, @@ -20,22 +22,17 @@ export const withRetry = async ( try { return await fn(...args) } catch (error: unknown) { - let retryAfter: number | undefined, statusCode: number + // dropbox specific error and retry handling if (error instanceof DropboxResponseError) { - const retryTime = error.headers.get('retry-after') || error.headers['retry-after'] - retryAfter = retryTime ? parseInt(retryTime, 10) : undefined - statusCode = error.status - } else { - const err = error as StatusableError - retryAfter = err.retryAfter - statusCode = err.status - } + const retryTime = error.headers.get('retry-after') + const retryAfter = retryTime ? parseInt(retryTime, 10) : undefined - if (statusCode === 429 && retryAfter) { - // If rate limit happens with retryAfter value from dropbox api. Wait - const waitMs = retryAfter * 1000 - console.warn(`Rate limited. Waiting for ${retryAfter} seconds before retry...`) - await new Promise((resolve) => setTimeout(resolve, waitMs)) + if (error.status === httpStatus.TOO_MANY_REQUESTS && retryAfter) { + // If rate limit happens with retryAfter value from dropbox api. Wait + const waitMs = retryAfter * 1000 + console.warn(`Rate limited. Waiting for ${retryAfter} seconds before retry...`) + await sleep(waitMs) + } } // Hopefully now sentry doesn't report retry errors as well. We have enough triage issues as it is @@ -65,15 +62,18 @@ export const withRetry = async ( factor: 2, // Exponential factor for timeout delay. Tweak this if issues still persist onFailedAttempt: (error: { error: unknown; attemptNumber: number; retriesLeft: number }) => { - console.warn( - 'Error from onFailedAttempt. Error: ', - JSON.stringify(error), - (error.error as StatusableError).status, - ) + if (error.error instanceof DropboxResponseError) { + const errorStatus = error.error.status + if ( + errorStatus !== httpStatus.TOO_MANY_REQUESTS && + errorStatus !== httpStatus.INTERNAL_SERVER_ERROR + ) + return + } if ( - (error.error as StatusableError).status !== 429 && - (error.error as StatusableError).status !== 500 + (error.error as StatusableError).status !== httpStatus.TOO_MANY_REQUESTS && + (error.error as StatusableError).status !== httpStatus.INTERNAL_SERVER_ERROR ) { return } @@ -82,14 +82,23 @@ export const withRetry = async ( error, ) }, - shouldRetry: (error: unknown) => { - // Typecasting because Copilot doesn't export an error class - const err = error as StatusableError + shouldRetry: (error: { error: unknown; attemptNumber: number; retriesLeft: number }) => { + if (error.error instanceof DropboxResponseError) { + const errorStatus = error.error.status + return ( + errorStatus === httpStatus.TOO_MANY_REQUESTS || + errorStatus === httpStatus.INTERNAL_SERVER_ERROR + ) + } - console.warn('Error from shouldRetry. Error: ', JSON.stringify(err), 'status: ', err.status) + // Typecasting because Copilot doesn't export an error class + const err = error.error as StatusableError // Retry only if statusCode indicates a ratelimit or Internal Server Error - return err.status === 429 || err.status === 500 + return ( + err.status === httpStatus.TOO_MANY_REQUESTS || + err.status === httpStatus.INTERNAL_SERVER_ERROR + ) }, }, ) diff --git a/src/utils/sleep.ts b/src/utils/sleep.ts new file mode 100644 index 0000000..ae79457 --- /dev/null +++ b/src/utils/sleep.ts @@ -0,0 +1 @@ +export const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) diff --git a/src/utils/withErrorHandler.ts b/src/utils/withErrorHandler.ts index f0640e9..282be7c 100644 --- a/src/utils/withErrorHandler.ts +++ b/src/utils/withErrorHandler.ts @@ -1,3 +1,4 @@ +import { DropboxResponseError } from 'dropbox' import httpStatus from 'http-status' import { type NextRequest, NextResponse } from 'next/server' import z, { ZodError } from 'zod' @@ -50,6 +51,11 @@ export const withErrorHandler = (handler: RequestHandler): RequestHandler => { } else if (error instanceof Error && error.message) { message = error.message logger.error('Error:', error) + } else if (error instanceof DropboxResponseError) { + message = error.error.error_summary || `DropboxResponseError: ${message}` + status = error.status + + logger.error('DropboxResponseError:', error.error) } else { message = 'Something went wrong' logger.error(error) From 0fa2b516a714379b9c0d69563c8a92242fe47a98 Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Tue, 3 Feb 2026 18:12:31 +0545 Subject: [PATCH 2/2] refactor(OUT-3045): make manualFetch function private to the class --- src/lib/dropbox/DropboxClient.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/dropbox/DropboxClient.ts b/src/lib/dropbox/DropboxClient.ts index 38fd90e..38069c6 100644 --- a/src/lib/dropbox/DropboxClient.ts +++ b/src/lib/dropbox/DropboxClient.ts @@ -55,7 +55,7 @@ export class DropboxClient { return this.clientInstance } - async _manualFetch( + private async manualFetch( url: string, headers?: Record, body?: NodeJS.ReadableStream | null, @@ -166,7 +166,6 @@ export class DropboxClient { }) } - manualFetch = this.wrapWithRetry(this._manualFetch) getAllFilesFolders = this.wrapWithRetry(this._getAllFilesFolders) downloadFile = this.wrapWithRetry(this._downloadFile) uploadFile = this.wrapWithRetry(this._uploadFile)