fix(RateLimiter): fix flakey tests

jest.config.cjs

@@ -3,5 +3,6 @@ const { createDefaultPreset } = require('ts-jest');
 /** @type {import("jest").Config} **/
 module.exports = {
 	testMatch: ['<rootDir>/lib/__tests__/**/*.test.ts'],
+	setupFilesAfterEnv: ['<rootDir>/lib/__tests__/testSetup.ts'],
 	...createDefaultPreset(),
 };

lib/RateLimiter/DefaultRateLimiter.ts

@@ -174,14 +174,13 @@ export default class DefaultRateLimiter extends BaseRateLimiter {
 	}
 
 	public async handleError(error: unknown): Promise<boolean> {
-		if (
-			axios.isAxiosError(error) &&
-			error.response &&
-			this.isRateLimitError(error.response)
-		) {
-			this.errorTimestamps.push(Date.now());
+		if (axios.isAxiosError(error) && error.response) {
+			if (this.isRateLimitError(error.response)) {
+				this.errorTimestamps.push(Date.now());
+			}
+
 			this.handleResult(error.response);
-			return true;
+			return this.isRateLimitError(error.response);
 		}
 
 		return false;

lib/__tests__/RateLimiter/DefaultRateLimiter.test.ts

@@ -2,15 +2,15 @@ import { jest, describe, expect, it } from '@jest/globals';
 
 import DefaultRateLimiter from '../../RateLimiter/DefaultRateLimiter';
 
-import { createRateLimitError, createResponse } from './util';
+import { createAxiosError, createResponse } from './util';
 
 class TestRateLimiter extends DefaultRateLimiter {
 	public setErrorTimestamps(timestamps: Array<number>) {
 		this.errorTimestamps = timestamps;
 	}
 
 	public async triggerError() {
-		return this.handleError(createRateLimitError());
+		return this.handleError(createAxiosError());
 	}
 
 	public getWaitTime() {
@@ -73,7 +73,7 @@ describe('DefaultRateLimiter', function () {
 			expect(limiter.getWaitTime()).toBe(1000);
 		});
 	});
-	describe('take', function () {
+	describe('wait', function () {
 		it("waits if there's no remaining requests", async function () {
 			jest.useFakeTimers();
 
@@ -88,16 +88,7 @@ describe('DefaultRateLimiter', function () {
 				}),
 			);
 
-			// set done to true after the wait time
-			let done = false;
-			const limiterPromise = limiter.wait().then(() => (done = true));
-			// make sure the short circuit case resolves, so we can test correctly
-			await jest.advanceTimersByTimeAsync(0);
-
-			expect(done).toBe(false);
-			await jest.advanceTimersByTimeAsync(1000);
-			await limiterPromise; // wait for the promise
-			expect(done).toBe(true);
+			await expect(limiter.wait()).toResolveAfterAtLeast(1000);
 		});
 		it('waits if a rate limit error was triggered', async function () {
 			jest.useFakeTimers();
@@ -106,28 +97,12 @@ describe('DefaultRateLimiter', function () {
 			// trigger an error to ratelimit the next request
 			await limiter.triggerError();
 
-			// set done to true after waiting for the ratelimiter
-			let done = false;
-			limiter.wait().then(() => (done = true));
-
-			// after 500 milliseconds we should still be waiting as our waitTime is 1000
-			await jest.advanceTimersByTimeAsync(500);
-			expect(done).toBe(false);
-
-			// after a second we should be done
-			await jest.advanceTimersByTimeAsync(500);
-			expect(done).toBe(true);
+			await expect(limiter.wait()).toResolveAfterAtLeast(1000);
 		});
 		it("immediately returns if there's no ratelimiting", async function () {
 			jest.useFakeTimers();
-
 			const limiter = new TestRateLimiter({ initialWaitTime: 1000 });
-			// set done to true after waiting for the ratelimiter
-			let done = false;
-			limiter.wait().then(() => (done = true));
-
-			await jest.advanceTimersByTimeAsync(0);
-			expect(done).toBe(true);
+			await expect(limiter.wait()).toResolveAfterAtLeast(0);
 		});
 		it('respects x-ratelimit-reset headers from API responses', async function () {
 			jest.useFakeTimers();
@@ -142,65 +117,48 @@ describe('DefaultRateLimiter', function () {
 				}),
 			);
 
-			// set done to true after waiting for the ratelimiter
-			let done = false;
-			const donePromise = limiter.wait().then(() => (done = true));
-
-			// assert it doesn't instantly gets set to true
-			await jest.advanceTimersByTimeAsync(0);
-			expect(done).toBe(false);
-
-			// should still be waiting after 59 seconds
-			await jest.advanceTimersByTimeAsync(59000);
-			expect(done).toBe(false);
-
-			// should be resolved after over 60 seconds total
-			await jest.advanceTimersByTimeAsync(1500);
-			expect(done).toBe(true);
-
-			await donePromise;
+			await expect(limiter.wait()).toResolveAfterAtLeast(60000);
 		});
 		it('respects x-ratelimit-reset headers from API errors', async function () {
 			jest.useFakeTimers();
 
 			const limiter = new TestRateLimiter();
 			await limiter.handleError(
-				createRateLimitError({
-					'x-ratelimit-remaining': 0,
-					'x-ratelimit-reset': Math.ceil(Date.now() / 1000) + 60,
+				createAxiosError(429, {
+					headers: {
+						'x-ratelimit-remaining': 0,
+						'x-ratelimit-reset': Math.ceil(Date.now() / 1000) + 60,
+					},
 				}),
 			);
 
-			// set done to true after waiting for the ratelimiter
-			let done = false;
-			const donePromise = limiter.wait().then(() => (done = true));
-
-			// assert it doesn't instantly gets set to true
-			await jest.advanceTimersByTimeAsync(0);
-			expect(done).toBe(false);
-
-			// should still be waiting after 59 seconds
-			await jest.advanceTimersByTimeAsync(59000);
-			expect(done).toBe(false);
-
-			// should be resolved after 60 seconds total
-			await jest.advanceTimersByTimeAsync(1500);
-			expect(done).toBe(true);
-
-			await donePromise;
+			await expect(limiter.wait()).toResolveAfterAtLeast(60000);
 		});
 	});
 	describe('handleError', function () {
 		it('returns true on errors related to ratelimiting', async function () {
 			const limiter = new TestRateLimiter();
-			expect(await limiter.handleError(createRateLimitError())).toBe(
-				true,
-			);
+			expect(await limiter.handleError(createAxiosError())).toBe(true);
 		});
 		it('returns false on errors unrelated to ratelimiting', async function () {
 			const limiter = new TestRateLimiter();
 			expect(await limiter.handleError('unrelated')).toBe(false);
 		});
-		it('respects the x-ratelimit-remaining header in API responses', async function () {});
+		it('respects the x-ratelimit-remaining header in API responses', async function () {
+			jest.useFakeTimers();
+
+			const limiter = new TestRateLimiter({
+				errorWindowBase: 5000,
+				initialWaitTime: 1000,
+			});
+			// trigger the ratelimiter by setting remaining requests to 0
+			await limiter.handleError(
+				createAxiosError(404, {
+					headers: { 'x-ratelimit-remaining': '0' },
+				}),
+			);
+
+			await expect(limiter.wait()).toResolveAfterAtLeast(1000);
+		});
 	});
 });

lib/__tests__/RateLimiter/NoOpRateLimiter.test.ts

@@ -2,11 +2,11 @@ import { jest, describe, expect, it } from '@jest/globals';
 
 import NoOpRateLimiter from '../../RateLimiter/NoOpRateLimiter';
 
-import { createResponse, createRateLimitError } from './util';
+import { createResponse, createAxiosError } from './util';
 
 describe('NoOpRateLimiter', function () {
 	describe('handleError', function () {
-		it.each([createRateLimitError(), 'random-value'])(
+		it.each([createAxiosError(), 'random-value'])(
 			'returns false with handleError(%j)',
 			async function (err) {
 				const limiter = new NoOpRateLimiter();

lib/__tests__/RateLimiter/util.ts

@@ -1,12 +1,19 @@
 import { AxiosResponse } from 'axios';
 
-export function createRateLimitError(headers = {}) {
+export function createAxiosError(
+	status = 429,
+	responseOptions: Partial<AxiosResponse> = {},
+) {
 	return {
 		isAxiosError: true,
+		code: status,
 		response: createResponse({
-			status: 429,
-			statusText: 'Too Many Requests',
-			headers,
+			status,
+			headers: {},
+			data: '',
+			statusText: '',
+			config: {},
+			...responseOptions,
 		}),
 	};
 }

lib/__tests__/extend-expect.d.ts

@@ -0,0 +1,5 @@
+declare module 'expect' {
+	interface Matchers<R> {
+		toResolveAfterAtLeast(value: number): Promise<R>;
+	}
+}

lib/__tests__/testSetup.ts

@@ -0,0 +1,39 @@
+import { jest, expect } from '@jest/globals';
+
+declare module 'expect' {
+	interface Matchers<R> {
+		toResolveAfterAtLeast(value: number): Promise<R>;
+	}
+}
+
+expect.extend({
+	async toResolveAfterAtLeast(received: Promise<any>, timePassedMs: number) {
+		let timeBefore = Date.now();
+		let resolvedAt: number | null = null;
+
+		received.then(() => (resolvedAt = Date.now()));
+		await jest.advanceTimersToNextTimerAsync();
+
+		if (resolvedAt === null) {
+			return {
+				pass: false,
+				message: () =>
+					`expected ${received} to resolve, but it never did`,
+			};
+		}
+
+		const actualTimePassed = resolvedAt - timeBefore;
+		if (!resolvedAt || resolvedAt - timeBefore < timePassedMs) {
+			return {
+				pass: false,
+				message: () =>
+					`expected ${received} to resolve after at least ${timePassedMs}ms, but it resolved in ${actualTimePassed}ms`,
+			};
+		}
+
+		return {
+			pass: true,
+			message: () => `${received} resolved after ${actualTimePassed}ms`,
+		};
+	},
+});