klaviyo-api-node icon indicating copy to clipboard operation
klaviyo-api-node copied to clipboard

Retry options broken in oauth branch

Open TheGame2500 opened this issue 1 year ago • 2 comments

Hello!

I've tried passing retry options to the segment APIs in both ways I'll document below, but to no avail. A 429 error will throw and no retries will be made.

I initially suspected that it's because of the newly introduced refreshAndRetry logic, but I commented out the entire try/catch block built around it and retrying still doesn't work.

The ways I've passed retry options to the oauth session:

private readonly retryOptions = new RetryOptions({
		numOfAttempts: 3,
		timeMultiple: 2,
		startingDelay: 1000,
	})

private readonly retryOptions = new RetryOptions(3, 5, 500) // btw this one has a TS validation error; says it expects only one argument

the way I'm using the segments api:

import {
	SegmentsApi,
	OAuthBasicSession,
	RetryOptions,
} from 'klaviyo-api'

export class KlaviyoService {
   	private readonly segmentsApi = SegmentsApi
	private readonly oAuthBasicSession = OAuthBasicSession
	
	async getSegmentProfileCount(
		oauthApiKey: string,
		segmentId: string,
	): Promise<number> {
		const session = new this.oAuthBasicSession(oauthApiKey, this.retryOptions) // where retry options is one of the 2 in the previous snippet
		const api = new this.segmentsApi(session)
		const response = await api.getSegment(segmentId, {
			additionalFieldsSegment: ['profile_count'],
		})
		return response.body.data.attributes.profileCount
	}

There are no identified faults in my code except that it's calling getSegmentProfileCount in quick succession. Sometimes it works, sometimes it fails with 429 instantly (no retries) on the 2nd call. I've validated this is the case by adding some logs and patching your npm package locally.

Cheers,

Gabriel

TheGame2500 avatar Feb 06 '24 17:02 TheGame2500

Thank you for reaching out. We will investigate this issue.

vpylypenko-klaviyo avatar Feb 06 '24 20:02 vpylypenko-klaviyo

We have been unable to recreate this issues, But you have pointed out some fixes and clarifications needed in our documention. Retry options does require an object like the one you mentioned above RetryOptions({ and the docs will be updated accordingly. Additionally numOfAttempts does actually correspond to the total number of attempts the api will try and make NOT the number of retries.

As a usage example of custom retry logic that looks similar to your check out:

test('tests that retry once for a total of 2 times',async () => {
    mockedAxios.mockRejectedValue({status: 429})
    const session = new OAuthBasicSession('fake-key', new RetryOptions({numOfAttempts: 2}))
    const accountsApi = new AccountsApi(session)
    try {
      await accountsApi.getAccount('fake_id')
    } catch (e) {
      expect(mockedAxios).toHaveBeenCalledTimes(2)
    }
  });

Ian-Montgomery-Klaviyo avatar Apr 22 '24 18:04 Ian-Montgomery-Klaviyo