From 929652fe14fe976ec3bbe2b67142d8ffbb3902f4 Mon Sep 17 00:00:00 2001 From: Johannes Klein Date: Wed, 30 Apr 2025 11:11:23 +0200 Subject: [PATCH] Add some context to 429 errors (#2861) * Add context to announcements API calls handleError * Add context to handleError for calls in comments * Add context to handleError for calls in computerVision * Add context to handleError for calls in flags * Add context to handleError for calls in identifications * Add context to handleError for calls in messages * Add context to handleError for calls in observations * Add context to handleError for calls in observationSounds * Add context to handleError for calls in places * Add context to handleError for calls in projects * Add context to handleError for calls in qualityMetrics * Add context to handleError for calls in relationships * Add context to handleError for calls in taxa * Add context to handleError for calls in translations * Add context to handleError for calls in users * Move creation of error context to the handleError function Also spread any other context that was passed into this function in the new context. * Move logger for 429 errors to the handleError function There are two codepaths that an error of 429 can follow here. Errors thrown from reactQueryRetry have a .status field whereas errors thrown from inaturalistjs have a .response.status field (and not the other in both cases). I don't want to update the handleError function for now but instead keep its codepaths the same, so I added logging for both paths. * Remove nullish values (null or undefined) from context --- src/api/announcements.js | 4 ++-- src/api/comments.js | 6 ++--- src/api/computerVision.ts | 2 +- src/api/error.js | 40 ++++++++++++++++++++++++++++----- src/api/flags.js | 2 +- src/api/identifications.js | 4 ++-- src/api/messages.ts | 2 +- src/api/observationSounds.js | 2 +- src/api/observations.js | 43 ++++++++++++++++++++---------------- src/api/places.js | 2 +- src/api/projects.js | 14 ++++++------ src/api/qualityMetrics.js | 6 ++--- src/api/relationships.js | 8 +++---- src/api/taxa.js | 4 ++-- src/api/translations.js | 2 +- src/api/users.js | 18 +++++++-------- src/sharedHelpers/logging.js | 25 +++------------------ 17 files changed, 99 insertions(+), 85 deletions(-) diff --git a/src/api/announcements.js b/src/api/announcements.js index 560994124..4712fd5ed 100644 --- a/src/api/announcements.js +++ b/src/api/announcements.js @@ -25,7 +25,7 @@ const searchAnnouncements = async ( params: Object = {}, opts: Object = {} ): Pr ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "searchAnnouncements", opts } } ); } }; @@ -33,7 +33,7 @@ const dismissAnnouncement = async ( params: Object = {}, opts: Object = {} ): Pr try { return await inatjs.announcements.dismiss( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "dismissAnnouncement", opts } } ); } }; diff --git a/src/api/comments.js b/src/api/comments.js index 0aa6a7083..9a09b5ccb 100644 --- a/src/api/comments.js +++ b/src/api/comments.js @@ -17,7 +17,7 @@ const createComment = async ( const { results } = await inatjs.comments.create( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createComment", opts } } ); } }; @@ -29,7 +29,7 @@ const updateComment = async ( const { results } = await inatjs.comments.update( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "updateComment", opts } } ); } }; @@ -41,7 +41,7 @@ const deleteComments = async ( const { results } = await inatjs.comments.delete( { id }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "deleteComments", id, opts } } ); } }; diff --git a/src/api/computerVision.ts b/src/api/computerVision.ts index 8bf3ce930..ac42f434e 100644 --- a/src/api/computerVision.ts +++ b/src/api/computerVision.ts @@ -20,7 +20,7 @@ const scoreImage = async ( try { return inatjs.computervision.score_image( { ...PARAMS, ...params }, opts ); } catch ( e ) { - return handleError( e as Error ); + return handleError( e as Error, { context: { functionName: "scoreImage", opts } } ); } }; diff --git a/src/api/error.js b/src/api/error.js index 555755b03..d4ed1ea39 100644 --- a/src/api/error.js +++ b/src/api/error.js @@ -40,18 +40,46 @@ export class INatApiTooManyRequestsError extends INatApiError { Object.defineProperty( INatApiTooManyRequestsError.prototype, "name", { value: "INatApiTooManyRequestsError" } ); +function createContext( e, options, extraContext ) { + const context = { + queryKey: options?.queryKey + ? JSON.stringify( options.queryKey ) + : "unknown", + failureCount: options?.failureCount, + timestamp: new Date().toISOString(), + errorType: e?.name || "Unknown", + status: e?.status || ( e.response + ? e.response.status + : null ), + url: e?.response?.url, + routeName: options?.routeName || e?.routeName, + routeParams: options?.routeParams || e?.routeParams, + ...( extraContext || {} ) + }; + // Remove nullish values (null or undefined) from context + return Object.fromEntries( + Object.entries( context ).filter( + ( [_, value] ) => value !== null && value !== undefined + ) + ); +} async function handleError( e: Object, options: Object = {} ): Object { + // Get context from options if available + const originalContext = options?.context || null; + const context = createContext( e, options, originalContext ); + if ( e.status === 429 ) { + logger.error( "429 without a response in handleError:", JSON.stringify( context ) ); + } + if ( !e.response ) { throw e; } - // Get context from options if available - const context = options?.context || null; - // 429 responses don't return JSON so the parsing that we do below will - // fail. Also, info about the request that triggered the 429 response is + // fail. Info about the request that triggered the 429 response is // kind of irrelevant. It's the behaviors that led up to being blocked that - // matter. + // matter. We log it anyhow. if ( e.response.status === 429 ) { + logger.error( "429 with a response in handleError:", JSON.stringify( context ) ); throw new INatApiTooManyRequestsError( context ); } @@ -83,7 +111,7 @@ async function handleError( e: Object, options: Object = {} ): Object { } ); } - const error = new INatApiError( errorJson, e.response.status, context ); + const error = new INatApiError( errorJson, e.response.status, originalContext ); // In theory code higher up in the stack will handle this error when thrown, // so it's probably not worth reporting at this stage. If it doesn't get diff --git a/src/api/flags.js b/src/api/flags.js index 5fad4929f..49fffbc69 100644 --- a/src/api/flags.js +++ b/src/api/flags.js @@ -16,7 +16,7 @@ const createFlag = async ( const { results } = await inatjs.flags.create( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createFlag", opts } } ); } }; diff --git a/src/api/identifications.js b/src/api/identifications.js index 45c44c988..f531412b8 100644 --- a/src/api/identifications.js +++ b/src/api/identifications.js @@ -17,7 +17,7 @@ const createIdentification = async ( const { results } = await inatjs.identifications.create( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createIdentification", opts } } ); } }; @@ -29,7 +29,7 @@ const updateIdentification = async ( const { results } = await inatjs.identifications.update( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "updateIdentification", opts } } ); } }; diff --git a/src/api/messages.ts b/src/api/messages.ts index 9996923cb..082270f89 100644 --- a/src/api/messages.ts +++ b/src/api/messages.ts @@ -19,7 +19,7 @@ const searchMessages = async ( params: Object = {}, opts: Object = {} ): Promise const { results } = await inatjs.messages.search( { ...PARAMS, ...params }, opts ); return results; } catch ( e ) { - return handleError( e as Error ); + return handleError( e as Error, { context: { functionName: "searchMessages", opts } } ); } }; diff --git a/src/api/observationSounds.js b/src/api/observationSounds.js index 8a4f6f1ba..1b93f951d 100644 --- a/src/api/observationSounds.js +++ b/src/api/observationSounds.js @@ -11,7 +11,7 @@ const deleteRemoteObservationSound = async ( try { return await inatjs.observation_sounds.delete( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "deleteRemoteObservationSound", opts } } ); } }; diff --git a/src/api/observations.js b/src/api/observations.js index c5fd5baae..d6069f02e 100644 --- a/src/api/observations.js +++ b/src/api/observations.js @@ -23,7 +23,7 @@ const searchObservations = async ( params: Object = {}, opts: Object = {} ): Pro response.results = response.results.map( mapToLocalSchema ); return response; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "searchObservations", opts } } ); } }; @@ -31,7 +31,7 @@ const faveObservation = async ( params: Object = {}, opts: Object = {} ): Promis try { return await inatjs.observations.fave( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "faveObservation", opts } } ); } }; @@ -39,7 +39,7 @@ const unfaveObservation = async ( params: Object = {}, opts: Object = {} ): Prom try { return await inatjs.observations.unfave( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "unfaveObservation", opts } } ); } }; @@ -61,7 +61,7 @@ const fetchRemoteObservation = async ( } return null; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchRemoteObservation", uuid, opts } } ); } }; @@ -83,7 +83,7 @@ const fetchRemoteObservations = async ( } return null; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchRemoteObservations", uuids, opts } } ); } }; @@ -91,7 +91,7 @@ const markAsReviewed = async ( params: Object = {}, opts: Object = {} ): Promise try { return await inatjs.observations.review( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "markAsReviewed", opts } } ); } }; @@ -102,7 +102,7 @@ const markObservationUpdatesViewed = async ( try { return await inatjs.observations.viewedUpdates( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "markObservationUpdatesViewed", opts } } ); } }; @@ -113,7 +113,7 @@ const createObservation = async ( try { return await inatjs.observations.create( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createObservation", opts } } ); } }; @@ -124,7 +124,7 @@ const updateObservation = async ( try { return await inatjs.observations.update( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "updateObservation", opts } } ); } }; @@ -140,7 +140,7 @@ const createOrUpdateEvidence = async ( try { return await apiEndpoint( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createOrUpdateEvidence", opts } } ); } }; @@ -152,7 +152,7 @@ const fetchObservationUpdates = async ( const { results } = await inatjs.observations.updates( params, opts ); return results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchObservationUpdates", opts } } ); } }; @@ -168,7 +168,12 @@ const fetchUnviewedObservationUpdatesCount = async ( }, opts ); return updatesCount; } catch ( e ) { - return handleError( e ); + return handleError( e, { + context: { + functionName: "fetchUnviewedObservationUpdatesCount", + opts + } + } ); } }; @@ -179,7 +184,7 @@ const deleteRemoteObservation = async ( try { return await inatjs.observations.delete( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "deleteRemoteObservation", opts } } ); } }; @@ -187,7 +192,7 @@ const fetchObservers = async ( params: Object = {} ) : Promise => { try { return inatjs.observations.observers( params ); } catch ( e ) { - return handleError( e, { throw: true } ); + return handleError( e, { context: { functionName: "fetchObservers" }, throw: true } ); } }; @@ -195,7 +200,7 @@ const fetchIdentifiers = async ( params: Object = {} ) : Promise => { try { return await inatjs.observations.identifiers( params ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchIdentifiers" } } ); } }; @@ -206,7 +211,7 @@ const fetchSpeciesCounts = async ( try { return inatjs.observations.speciesCounts( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchSpeciesCounts", opts } } ); } }; @@ -217,7 +222,7 @@ const checkForDeletedObservations = async ( try { return await inatjs.observations.deleted( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "checkForDeletedObservations", opts } } ); } }; @@ -228,7 +233,7 @@ const fetchSubscriptions = async ( try { return inatjs.observations.subscriptions( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchSubscriptions", opts } } ); } }; @@ -239,7 +244,7 @@ const createSubscription = async ( try { return inatjs.observations.subscribe( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "createSubscription", opts } } ); } }; diff --git a/src/api/places.js b/src/api/places.js index 069741a12..af29cc518 100644 --- a/src/api/places.js +++ b/src/api/places.js @@ -15,7 +15,7 @@ const fetchPlace = async ( ? results[0] : null; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchPlace", id, opts } } ); } }; diff --git a/src/api/projects.js b/src/api/projects.js index 8fbf7cbe9..7ba9536b8 100644 --- a/src/api/projects.js +++ b/src/api/projects.js @@ -23,7 +23,7 @@ const fetchProjects = async ( const { results } = await inatjs.projects.fetch( id, params, opts ); return results[0]; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchProjects", id, opts } } ); } }; @@ -34,7 +34,7 @@ const fetchProjectMembers = async ( try { return await inatjs.projects.members( params, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchProjectMembers", opts } } ); } }; @@ -46,7 +46,7 @@ const fetchProjectPosts = async ( const response = await inatjs.projects.posts( params, opts ); return response.total_results; } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "fetchProjectPosts", opts } } ); } }; @@ -54,7 +54,7 @@ const searchProjects = async ( params: Object = {}, opts: Object = {} ): Promise try { return await inatjs.projects.search( { ...PARAMS, ...params }, opts ); } catch ( e ) { - return handleError( e ); + return handleError( e, { context: { functionName: "searchProjects", opts } } ); } }; @@ -62,7 +62,7 @@ const joinProject = async ( params: Object = {}, opts: Object = {} ): Promise { console.log( apiError, "API error in reactQueryRetry handleTooManyRequestsErrors" ); - } + }, + failureCount, + ...options } ); return shouldRetry;