diff --git a/src/index.ts b/src/index.ts index 3b97df6a..18220653 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,8 +109,9 @@ export function throttling(octokit: Octokit, octokitOptions = {}) { // @ts-ignore state.retryLimiter.on("failed", async function (error, info) { const options = info.args[info.args.length - 1]; + const { pathname } = new URL(options.url, "http://github.test"); const shouldRetryGraphQL = - options.url.startsWith("/graphql") && error.status !== 401; + pathname.startsWith("/graphql") && error.status !== 401; if (!(shouldRetryGraphQL || error.status === 403)) { return; diff --git a/src/wrap-request.ts b/src/wrap-request.ts index ab8fae1d..0421bb84 100644 --- a/src/wrap-request.ts +++ b/src/wrap-request.ts @@ -8,9 +8,9 @@ export function wrapRequest(state, request, options) { // @ts-ignore async function doRequest(state, request, options) { const isWrite = options.method !== "GET" && options.method !== "HEAD"; - const isSearch = - options.method === "GET" && options.url.startsWith("/search/"); - const isGraphQL = options.url.startsWith("/graphql"); + const { pathname } = new URL(options.url, "http://github.test"); + const isSearch = options.method === "GET" && pathname.startsWith("/search/"); + const isGraphQL = pathname.startsWith("/graphql"); const retryCount = ~~options.request.retryCount; const jobOptions = retryCount > 0 ? { priority: 0, weight: 0 } : {}; @@ -28,7 +28,7 @@ async function doRequest(state, request, options) { } // Guarantee at least 3000ms between requests that trigger notifications - if (isWrite && state.triggersNotification(options.url)) { + if (isWrite && state.triggersNotification(pathname)) { await state.notifications.key(state.id).schedule(jobOptions, noop); } diff --git a/test/plugin.test.ts b/test/plugin.test.ts index c616bcc6..8fa1d03e 100644 --- a/test/plugin.test.ts +++ b/test/plugin.test.ts @@ -232,34 +232,48 @@ describe("Github API best practices", function () { }, }); - const req1 = octokit.request("GET /search/route1", { + const req1 = octokit.request("GET /search/route", { request: { - responses: [{ status: 201, headers: {}, data: {} }], - }, - }); - const req2 = octokit.request("GET /route2", { - request: { - responses: [{ status: 202, headers: {}, data: {} }], + responses: [{ status: 200, headers: {}, data: {} }], }, }); - const req3 = octokit.request("GET /search/route3", { + const req2 = octokit.request("GET /other-route", { request: { - responses: [{ status: 203, headers: {}, data: {} }], + responses: [{ status: 200, headers: {}, data: {} }], }, }); + const req3 = octokit.request( + "GET https://api.github.com/search/route?page=2", + { + request: { + responses: [{ status: 200, headers: {}, data: {} }], + }, + } + ); + const req4 = octokit.request( + "GET https://api.github.com/search/route?page=3", + { + request: { + responses: [{ status: 200, headers: {}, data: {} }], + }, + } + ); - await Promise.all([req1, req2, req3]); + await Promise.all([req1, req2, req3, req4]); expect(octokit.__requestLog).toStrictEqual([ - "START GET /route2", - "END GET /route2", - "START GET /search/route1", - "END GET /search/route1", - "START GET /search/route3", - "END GET /search/route3", + "START GET /other-route", + "END GET /other-route", + "START GET /search/route", + "END GET /search/route", + "START GET https://api.github.com/search/route?page=2", + "END GET https://api.github.com/search/route?page=2", + "START GET https://api.github.com/search/route?page=3", + "END GET https://api.github.com/search/route?page=3", ]); - expect( - octokit.__requestTimings[4] - octokit.__requestTimings[2] - ).toBeLessThan(70); + + const ms = octokit.__requestTimings[4] - octokit.__requestTimings[2]; + expect(ms).toBeLessThan(70); + expect(ms).toBeGreaterThan(30); }); it("Should optimize throughput rather than maintain ordering", async function () { diff --git a/test/retry.test.ts b/test/retry.test.ts index 1a0a78bd..6dc9a297 100644 --- a/test/retry.test.ts +++ b/test/retry.test.ts @@ -213,6 +213,56 @@ describe("Retry", function () { expect(ms).toBeGreaterThan(30); }); + it("Should work with full URL", async function () { + let eventCount = 0; + const octokit = new TestOctokit({ + throttle: { + write: new Bottleneck.Group({ minTime: 50 }), + onRateLimit: (retryAfter: number, options: object) => { + expect(options).toMatchObject({ + method: "POST", + url: "https://api.github.com/graphql", + request: { retryCount: eventCount }, + }); + expect(retryAfter).toEqual(0); + eventCount++; + return true; + }, + onAbuseLimit: () => 1, + }, + }); + + const res = await octokit.request("POST https://api.github.com/graphql", { + request: { + responses: [ + { + status: 200, + headers: { + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": "123", + }, + data: { errors: [{ type: "RATE_LIMITED" }] }, + }, + { status: 200, headers: {}, data: { message: "Yay!" } }, + ], + }, + }); + + expect(res.status).toEqual(200); + expect(res.data).toMatchObject({ message: "Yay!" }); + expect(eventCount).toEqual(1); + expect(octokit.__requestLog).toStrictEqual([ + "START POST https://api.github.com/graphql", + "END POST https://api.github.com/graphql", + "START POST https://api.github.com/graphql", + "END POST https://api.github.com/graphql", + ]); + + const ms = octokit.__requestTimings[2] - octokit.__requestTimings[0]; + expect(ms).toBeLessThan(70); + expect(ms).toBeGreaterThan(30); + }); + it("Should ignore other error types", async function () { let eventCount = 0; const octokit = new TestOctokit({