authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-11 10:35:54-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-11 10:35:54-07:00
logf23d0084b6e023ffb1af9e332d8666affc99d542
tree29739400873ad46de31f497aad13ae9034ba7620
parent8ac6846a0b855c3aad70adbf745ff113d4eb3f1f
signature Signed by PGP key B5690EEEBB952194

perf(github): resolve credentials once per GraphQL request (#18289)

2 files changed, 65 insertions(+), 33 deletions(-)

packages/source-control-github/src/server/GitHubApi.test.ts+44-8
......@@ -1,6 +1,5 @@
11import { describe, expect, it } from "@effect/vitest";
22import * as NodeCrypto from "@effect/platform-node/NodeCrypto";
3import * as NodeServices from "@effect/platform-node/NodeServices";
43import * as Effect from "effect/Effect";
54import * as Layer from "effect/Layer";
65import * as Redacted from "effect/Redacted";
......@@ -24,16 +23,20 @@ function harness(respond: (request: HttpClientRequest.HttpClientRequest) => Resp
2423 const requests: Array<HttpClientRequest.HttpClientRequest> = [];
2524 let tokens = ["first", "second"];
2625 let invalidations = 0;
26 let credentialReads = 0;
2727 const credentials = Layer.succeed(
2828 GitHubCredentials.GitHubCredentials,
2929 GitHubCredentials.GitHubCredentials.of({
3030 get: (host) =>
31 Effect.sync(() => ({
32 host,
33 token: Redacted.make(tokens[0]!),
34 source: "gh" as const,
35 fingerprint: `${host}:${tokens[0]}`,
36 })),
31 Effect.sync(() => {
32 credentialReads++;
33 return {
34 host,
35 token: Redacted.make(tokens[0]!),
36 source: "gh" as const,
37 fingerprint: `${host}:${tokens[0]}`,
38 };
39 }),
3740 invalidate: () =>
3841 Effect.sync(() => {
3942 invalidations++;
......@@ -53,7 +56,12 @@ function harness(respond: (request: HttpClientRequest.HttpClientRequest) => Resp
5356 Layer.provideMerge(GitHubQuota.layer),
5457 Layer.provideMerge(SourceControlRateLimit.layer),
5558 );
56 return { layer, requests, invalidations: () => invalidations };
59 return {
60 layer,
61 requests,
62 invalidations: () => invalidations,
63 credentialReads: () => credentialReads,
64 };
5765}
5866
5967const json = (body: unknown, init?: ResponseInit) =>
......@@ -119,6 +127,34 @@ describe("GitHubApi", () => {
119127 }).pipe(Effect.provide(layer));
120128 });
121129
130 it.effect("resolves GraphQL credentials once per request and refreshes after refusal", () => {
131 const { layer, requests, invalidations, credentialReads } = harness((request) =>
132 request.headers.authorization === "Bearer first"
133 ? new Response(null, { status: 401 })
134 : json({ data: { viewer: { login: "julius" } } }),
135 );
136 return Effect.gen(function* () {
137 const api = yield* GitHubApi.GitHubApi;
138 const input = {
139 host: "github.com",
140 operation: "viewer",
141 query: "query { viewer { login } }",
142 };
143 const refused = yield* Effect.flip(api.graphql(input));
144 expect(refused._tag).toBe("GitHubApiAuthenticationError");
145 expect(credentialReads()).toBe(1);
146 expect(invalidations()).toBe(1);
147
148 const body = yield* api.graphql(input);
149 expect(body).toContain('"login":"julius"');
150 expect(credentialReads()).toBe(2);
151 expect(requests.map((request) => request.headers.authorization)).toEqual([
152 "Bearer first",
153 "Bearer second",
154 ]);
155 }).pipe(Effect.provide(layer));
156 });
157
122158 it.effect(
123159 "traces the operation, path, query and quota, never the query string or variables",
124160 () => {
packages/source-control-github/src/server/GitHubApi.ts+21-25
......@@ -534,8 +534,6 @@ export const make = Effect.gen(function* () {
534534 const graphql: GitHubApi["Service"]["graphql"] = Effect.fn("GitHubApi.graphql")(
535535 function* (input) {
536536 const host = normalizeHost(input.host);
537 const { fingerprint } = yield* credential(host);
538 const scope = (yield* SourceControlRateLimit.CredentialScope) || fingerprint;
539537 const allowReserve = input.allowReserve ?? (yield* SourceControlRateLimit.Interactive);
540538 // The document, never its variables: user text (bodies, search terms) travels as variables.
541539 yield* Effect.annotateCurrentSpan({
......@@ -546,32 +544,30 @@ export const make = Effect.gen(function* () {
546544 ? `${input.query.slice(0, TRACED_QUERY_MAX_CHARS)}…`
547545 : input.query,
548546 });
549 return yield* Effect.gen(function* () {
550 const response = yield* send({
547 const response = yield* send({
548 host,
549 operation: input.operation,
550 request: HttpClientRequest.post(gitHubApiUrls(host).graphql).pipe(
551 HttpClientRequest.acceptJson,
552 HttpClientRequest.bodyJsonUnsafe({
553 query: input.query,
554 variables: input.variables ?? {},
555 }),
556 ),
557 maxResponseBytes: input.maxResponseBytes ?? DEFAULT_MAX_RESPONSE_BYTES,
558 allowReserve,
559 acceptNotModified: false,
560 graphql: true,
561 });
562 // A body cut at the byte cap hides any `errors` past the cut and cannot be decoded.
563 if (response.truncated) {
564 return yield* new GitHubApiResponseError({
551565 host,
552566 operation: input.operation,
553 request: HttpClientRequest.post(gitHubApiUrls(host).graphql).pipe(
554 HttpClientRequest.acceptJson,
555 HttpClientRequest.bodyJsonUnsafe({
556 query: input.query,
557 variables: input.variables ?? {},
558 }),
559 ),
560 maxResponseBytes: input.maxResponseBytes ?? DEFAULT_MAX_RESPONSE_BYTES,
561 allowReserve,
562 acceptNotModified: false,
563 graphql: true,
567 status: response.status,
564568 });
565 // A body cut at the byte cap hides any `errors` past the cut and cannot be decoded.
566 if (response.truncated) {
567 return yield* new GitHubApiResponseError({
568 host,
569 operation: input.operation,
570 status: response.status,
571 });
572 }
573 return response.body;
574 }).pipe(Effect.provideService(SourceControlRateLimit.CredentialScope, scope));
569 }
570 return response.body;
575571 },
576572 );
577573