Skip to content

Commit ea4d56d

Browse files
sanjaibalajeesanjaibalajee
committed
fix(pull-requests): retain failed host reads for fallback
1 parent 6c23b8c commit ea4d56d

10 files changed

Lines changed: 439 additions & 54 deletions

File tree

‎apps/server/src/pullRequest/PullRequestService.test.ts‎

Lines changed: 152 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ import * as SourceControlProviderRegistry from "../sourceControl/SourceControlPr
3636
import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit";
3737
import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli";
3838
import * as ForgejoPullRequestProvider from "@t3tools/source-control-forgejo/server/ForgejoPullRequestProvider";
39+
import * as BitbucketApi from "@t3tools/source-control-bitbucket/server/BitbucketApi";
40+
import * as BitbucketPullRequestApi from "@t3tools/source-control-bitbucket/server/BitbucketPullRequestApi";
41+
import * as BitbucketPullRequestProvider from "@t3tools/source-control-bitbucket/server/BitbucketPullRequestProvider";
3942
import * as GitHubRest from "@t3tools/source-control-github/server/GitHubRest";
4043
import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials";
4144
import * as GitHubPullRequestApi from "@t3tools/source-control-github/server/GitHubPullRequestApi";
@@ -1287,26 +1290,161 @@ it.effect("keeps one host listed when another is not set up", () =>
12871290
}),
12881291
);
12891292

1290-
it.effect("fails as unavailable only when no host can be read", () =>
1293+
it.effect("retains an unavailable host's summary when it is the only queried host", () =>
1294+
Effect.gen(function* () {
1295+
for (const reason of ["missing-tool", "unauthenticated"] as const) {
1296+
const service = yield* makeService({
1297+
projects: [
1298+
project({
1299+
id: "p1",
1300+
title: "t3code",
1301+
workspaceRoot: "/a",
1302+
repository: "pingdotgg/t3code",
1303+
}),
1304+
],
1305+
providers: [
1306+
fakeProvider(SourceControlProviderKind.make("github"), {
1307+
getViewer: () =>
1308+
Effect.fail(unusable(SourceControlProviderKind.make("github"), reason)),
1309+
}),
1310+
],
1311+
});
1312+
1313+
for (const filter of [{}, { host: "github.com" }]) {
1314+
const error = yield* service.list({ state: "open", ...filter }).pipe(Effect.flip);
1315+
assert.strictEqual(error._tag, "PullRequestUnavailableError");
1316+
assert.strictEqual(
1317+
error._tag === "PullRequestUnavailableError" ? error.reason : null,
1318+
reason === "missing-tool" ? "cli-missing" : "cli-unauthenticated",
1319+
);
1320+
assert.isDefined(error.listResult);
1321+
const result = error.listResult!;
1322+
assert.deepStrictEqual(result.entries, []);
1323+
assert.deepStrictEqual(result.viewers, {});
1324+
assert.deepStrictEqual(
1325+
result.providers.map(({ host, configured, projectIds }) => ({
1326+
host,
1327+
configured,
1328+
projectIds: projectIds?.map(String),
1329+
})),
1330+
[{ host: "github.com", configured: false, projectIds: ["p1"] }],
1331+
);
1332+
assert.strictEqual(result.providers[0]?.detail?.includes("GitHub"), true);
1333+
assert.deepStrictEqual(
1334+
result.errors.map(({ projectId }) => projectId),
1335+
["p1"],
1336+
);
1337+
}
1338+
}
1339+
}),
1340+
);
1341+
1342+
it.effect(
1343+
"retains a Bitbucket host summary when saved credentials are rejected with HTTP 403",
1344+
() =>
1345+
Effect.gen(function* () {
1346+
const provider = yield* BitbucketPullRequestProvider.make.pipe(
1347+
Effect.provide(
1348+
Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({
1349+
getViewer: () =>
1350+
Effect.fail(
1351+
new BitbucketApi.BitbucketResponseError({
1352+
operation: "request",
1353+
status: 403,
1354+
responseBodyLength: 0,
1355+
}),
1356+
),
1357+
}),
1358+
),
1359+
);
1360+
const service = yield* makeService({
1361+
projects: [
1362+
project({
1363+
id: "bitbucket",
1364+
title: "Widget",
1365+
workspaceRoot: "/widget",
1366+
repository: "acme/widget",
1367+
provider: "bitbucket",
1368+
host: "bitbucket.org",
1369+
}),
1370+
],
1371+
providers: [provider],
1372+
});
1373+
for (const filter of [{}, { host: "bitbucket.org" }]) {
1374+
const error = yield* service.list({ state: "open", ...filter }).pipe(Effect.flip);
1375+
assert.strictEqual(error._tag, "PullRequestOperationError");
1376+
assert.isDefined(error.listResult);
1377+
const result = error.listResult!;
1378+
assert.deepStrictEqual(
1379+
result.providers.map(({ host, configured, projectIds }) => ({
1380+
host,
1381+
configured,
1382+
projectIds: projectIds?.map(String),
1383+
})),
1384+
[{ host: "bitbucket.org", configured: false, projectIds: ["bitbucket"] }],
1385+
);
1386+
assert.deepStrictEqual(result.entries, []);
1387+
assert.deepStrictEqual(
1388+
result.errors.map(({ projectId }) => projectId),
1389+
["bitbucket"],
1390+
);
1391+
}
1392+
}),
1393+
);
1394+
1395+
it.effect("identifies every SSH worktree behind an unavailable refined Forgejo authority", () =>
12911396
Effect.gen(function* () {
12921397
const service = yield* makeService({
1293-
projects: [
1294-
project({ id: "p1", title: "t3code", workspaceRoot: "/a", repository: "pingdotgg/t3code" }),
1295-
],
1398+
projects: ["ssh", "ssh-worktree"].map((id) =>
1399+
project({
1400+
id,
1401+
title: id,
1402+
workspaceRoot: `/${id}`,
1403+
repository: "team/repo",
1404+
provider: "forgejo",
1405+
host: "ssh.forge.example",
1406+
remoteUrl: "ssh://[email protected]:2222/team/repo.git",
1407+
}),
1408+
),
12961409
providers: [
1297-
fakeProvider(SourceControlProviderKind.make("github"), {
1298-
getViewer: () =>
1299-
Effect.fail(unusable(SourceControlProviderKind.make("github"), "missing-tool")),
1410+
fakeProvider(SourceControlProviderKind.make("forgejo"), {
1411+
getViewer: ({ host }) => {
1412+
assert.strictEqual(host, "forge.example:3000");
1413+
return Effect.fail(
1414+
unusable(SourceControlProviderKind.make("forgejo"), "unauthenticated"),
1415+
);
1416+
},
1417+
listChangeRequests: () => Effect.die("An unauthenticated host must not be queried"),
13001418
}),
13011419
],
1420+
resolveHandle: ({ context }) =>
1421+
Effect.succeed({
1422+
context: {
1423+
...context!,
1424+
provider: {
1425+
kind: SourceControlProviderKind.make("forgejo"),
1426+
name: "Forgejo",
1427+
baseUrl: "http://forge.example:3000",
1428+
},
1429+
},
1430+
provider: undefined as never,
1431+
}),
13021432
});
1303-
13041433
const error = yield* service.list({ state: "open" }).pipe(Effect.flip);
1305-
1306-
assert.strictEqual(error._tag, "PullRequestUnavailableError");
1307-
assert.strictEqual(
1308-
error._tag === "PullRequestUnavailableError" ? error.reason : null,
1309-
"cli-missing",
1434+
assert.isDefined(error.listResult);
1435+
const result = error.listResult!;
1436+
assert.deepStrictEqual(
1437+
result.providers.map(({ host, configured, projectIds }) => ({
1438+
host,
1439+
configured,
1440+
projectIds: projectIds?.map(String),
1441+
})),
1442+
[{ host: "forge.example:3000", configured: false, projectIds: ["ssh", "ssh-worktree"] }],
1443+
);
1444+
assert.deepStrictEqual(result.entries, []);
1445+
assert.deepStrictEqual(
1446+
result.errors.map(({ projectId }) => projectId),
1447+
["ssh"],
13101448
);
13111449
}),
13121450
);
@@ -1342,6 +1480,7 @@ it.effect("reads a repository once when several worktrees share it", () =>
13421480

13431481
assert.strictEqual(calls, 1);
13441482
assert.strictEqual(result.entries.length, 1);
1483+
assert.deepStrictEqual(result.providers[0]?.projectIds?.map(String), ["p1", "p2"]);
13451484
}),
13461485
);
13471486

‎apps/server/src/pullRequest/PullRequestService.ts‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ interface WorkspaceProjects {
318318
{ readonly kind: SourceControlProviderKind; readonly projectCount: number }
319319
>;
320320
readonly viewerRoots: ReadonlyMap<string, ReadonlyArray<string>>;
321+
readonly projectIdsByHost: ReadonlyMap<string, ReadonlyArray<ProjectId>>;
321322
}
322323

323324
interface RepositoryBatch {
@@ -705,6 +706,7 @@ export const make = Effect.gen(function* () {
705706
{ kind: SourceControlProviderKind; projectCount: number }
706707
>();
707708
const viewerRoots = new Map<string, string[]>();
709+
const projectIdsByHost = new Map<string, ProjectId[]>();
708710
const seen = new Set<string>();
709711
for (const project of projects) {
710712
if (filter.projectId !== undefined && project.id !== filter.projectId) continue;
@@ -729,6 +731,9 @@ export const make = Effect.gen(function* () {
729731
if (filter.host !== undefined && host !== filter.host.toLowerCase()) {
730732
continue;
731733
}
734+
const projectIds = projectIdsByHost.get(host);
735+
if (projectIds === undefined) projectIdsByHost.set(host, [project.id]);
736+
else projectIds.push(project.id);
732737
const api = registry.get(kind);
733738
if (api !== null) {
734739
const roots = viewerRoots.get(host);
@@ -757,7 +762,7 @@ export const make = Effect.gen(function* () {
757762
repositoryKey === null ? null : canonicalRepositoryKey(repositoryKey.toLowerCase()),
758763
});
759764
}
760-
return { supported, unimplemented, viewerRoots };
765+
return { supported, unimplemented, viewerRoots, projectIdsByHost };
761766
}),
762767
);
763768

@@ -1083,6 +1088,7 @@ export const make = Effect.gen(function* () {
10831088
supported: projects,
10841089
unimplemented,
10851090
viewerRoots,
1091+
projectIdsByHost,
10861092
} = yield* listWorkspaceProjects(input);
10871093
const projectCounts = new Map<string, number>();
10881094
for (const { host } of projects) {
@@ -1105,6 +1111,7 @@ export const make = Effect.gen(function* () {
11051111
searchesOnHost: capabilities?.search ?? false,
11061112
actions: capabilities?.actions ?? [],
11071113
projectCount: projectCounts.get(result.host) ?? 1,
1114+
projectIds: projectIdsByHost.get(result.host) ?? [],
11081115
configured: result.viewer !== null,
11091116
detail: result.error === null ? null : providerDetail(result.error),
11101117
};
@@ -1115,6 +1122,7 @@ export const make = Effect.gen(function* () {
11151122
searchesOnHost: false,
11161123
actions: [],
11171124
projectCount,
1125+
projectIds: projectIdsByHost.get(host) ?? [],
11181126
configured: false,
11191127
detail: "This host cannot be browsed here yet.",
11201128
})),
@@ -1133,24 +1141,28 @@ export const make = Effect.gen(function* () {
11331141
message: `${repository} could not be read.`,
11341142
}));
11351143
if (readable.length === 0) {
1144+
const listResult: PullRequestListResult = {
1145+
viewers: viewers as PullRequestListResult["viewers"],
1146+
providers,
1147+
entries: [],
1148+
errors: unreadable,
1149+
truncated: false,
1150+
nextCursors: {},
1151+
};
11361152
const errors = viewerResults.flatMap((result) =>
11371153
result.error === null || !selected.some(({ host }) => host === result.host)
11381154
? []
11391155
: [result.error],
11401156
);
11411157
const blocking = errors.find(isProviderUnusable) ?? errors[0];
11421158
if (blocking) {
1143-
return yield* toPullRequestError("list")(blocking);
1159+
// Preserve failure/retry semantics while letting clients try another server for these hosts.
1160+
const error = toPullRequestError("list")(blocking);
1161+
return yield* error._tag === "PullRequestUnavailableError"
1162+
? new PullRequestUnavailableError({ ...error, listResult })
1163+
: new PullRequestOperationError({ ...error, listResult });
11441164
}
1145-
1146-
return {
1147-
viewers: viewers as PullRequestListResult["viewers"],
1148-
providers,
1149-
entries: [],
1150-
errors: [],
1151-
truncated: false,
1152-
nextCursors: {},
1153-
};
1165+
return listResult;
11541166
}
11551167

11561168
const limit = input.limit ?? DEFAULT_REPOSITORY_LIST_LIMIT;

‎apps/web/src/components/pullRequest/pullRequestProjectAssignment.browser.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ function answer(environmentId: EnvironmentId): PullRequestListResult {
116116
kind: "bitbucket" as never,
117117
searchesOnHost: false,
118118
projectCount: 1,
119+
projectIds: [projectId],
119120
configured,
120121
detail: configured ? null : "Bitbucket rejected the configured credentials.",
121122
},

0 commit comments

Comments
 (0)