Skip to content

Commit 668ffcb

Browse files
Merge pull request #511 from corbitsdev/cl-7247-credential-auth-report-error
CL-7247 batch 1: route packages/connections catches through reportError
2 parents ee436e0 + b1adb7e commit 668ffcb

28 files changed

Lines changed: 630 additions & 38 deletions

‎packages/connections/src/connected-hook.test.ts‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { describe, expect, spyOn, test } from "bun:test";
2+
import * as errorSink from "@corbits/error-sink";
23
import {
4+
fireConnectedHook,
35
fireInferenceCredentialSeedableHook,
46
type InferenceCredentialSeedableInfo,
7+
type ServiceConnectedInfo,
58
} from "./connected-hook";
69

710
function seedableInfo(): InferenceCredentialSeedableInfo {
@@ -15,6 +18,46 @@ function seedableInfo(): InferenceCredentialSeedableInfo {
1518
};
1619
}
1720

21+
function connectedInfo(): ServiceConnectedInfo {
22+
return {
23+
tenantId: "tenant_1",
24+
principalId: "principal_1",
25+
connectorId: "github",
26+
displayName: "GitHub",
27+
};
28+
}
29+
30+
describe("fireConnectedHook", () => {
31+
test("does nothing when no hook is wired", async () => {
32+
await expect(
33+
fireConnectedHook(undefined, () => {}, connectedInfo()),
34+
).resolves.toBeUndefined();
35+
});
36+
37+
test("logs and reports a hook failure rather than breaking the connect", async () => {
38+
const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test");
39+
const logged: string[] = [];
40+
await expect(
41+
fireConnectedHook(
42+
() => {
43+
throw new Error("card settle unavailable");
44+
},
45+
(line) => logged.push(line),
46+
connectedInfo(),
47+
),
48+
).resolves.toBeUndefined();
49+
expect(logged).toHaveLength(1);
50+
expect(report).toHaveBeenCalledTimes(1);
51+
expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error);
52+
expect(report.mock.calls[0]?.[1]).toMatchObject({
53+
operation: "fire_connected_hook",
54+
tenantId: "tenant_1",
55+
extra: { connectorId: "github" },
56+
});
57+
report.mockRestore();
58+
});
59+
});
60+
1861
describe("fireInferenceCredentialSeedableHook", () => {
1962
test("does nothing when no hook is wired", async () => {
2063
await expect(
@@ -34,7 +77,8 @@ describe("fireInferenceCredentialSeedableHook", () => {
3477
expect(calls).toEqual([seedableInfo()]);
3578
});
3679

37-
test("logs and swallows a hook failure rather than breaking the connect", async () => {
80+
test("logs and reports a hook failure rather than breaking the connect", async () => {
81+
const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test");
3882
const logged: string[] = [];
3983
await expect(
4084
fireInferenceCredentialSeedableHook(
@@ -48,5 +92,13 @@ describe("fireInferenceCredentialSeedableHook", () => {
4892
expect(logged).toHaveLength(1);
4993
expect(logged[0]).toContain("tenant_1");
5094
expect(logged[0]).toContain("drain unavailable");
95+
expect(report).toHaveBeenCalledTimes(1);
96+
expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error);
97+
expect(report.mock.calls[0]?.[1]).toMatchObject({
98+
operation: "fire_inference_credential_seedable_hook",
99+
tenantId: "tenant_1",
100+
extra: { provider: "ollama" },
101+
});
102+
report.mockRestore();
51103
});
52104
});

‎packages/connections/src/connected-hook.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// without knowing which door the connection came through. Best-effort
66
// by contract: a hook failure is logged and never breaks the connect
77
// itself — the credential is already stored when this fires.
8+
import { reportError } from "@corbits/error-sink";
89

910
export type ServiceConnectedInfo = {
1011
readonly tenantId: string;
@@ -30,6 +31,11 @@ export async function fireConnectedHook(
3031
log(
3132
`onConnected hook failed for ${info.connectorId} on tenant ${info.tenantId}: ${message}`,
3233
);
34+
reportError(cause, {
35+
operation: "fire_connected_hook",
36+
tenantId: info.tenantId,
37+
extra: { connectorId: info.connectorId },
38+
});
3339
}
3440
}
3541

@@ -77,5 +83,10 @@ export async function fireInferenceCredentialSeedableHook(
7783
log(
7884
`onInferenceCredentialUsable hook failed for ${info.provider} on tenant ${info.tenantId}: ${message}`,
7985
);
86+
reportError(cause, {
87+
operation: "fire_inference_credential_seedable_hook",
88+
tenantId: info.tenantId,
89+
extra: { provider: info.provider },
90+
});
8091
}
8192
}

‎packages/connections/src/github-connect.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
// surface GitHub's own `error`/`error_description` shape (distinct from
44
// a non-2xx status — GitHub's token endpoint answers 200 with an `error`
55
// field on a rejected code).
6-
import { describe, expect, test } from "bun:test";
6+
import { describe, expect, spyOn, test } from "bun:test";
7+
import * as errorSink from "@corbits/error-sink";
78
import {
89
exchangeCodeForGithubToken,
910
type ExchangeFetch,
@@ -101,6 +102,7 @@ describe("exchangeCodeForGithubToken", () => {
101102
});
102103

103104
test("a transport failure is reported honestly", async () => {
105+
const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test");
104106
const fetchImpl: ExchangeFetch = async () => {
105107
throw new Error("getaddrinfo ENOTFOUND");
106108
};
@@ -118,5 +120,11 @@ describe("exchangeCodeForGithubToken", () => {
118120
expect(result.message).toContain("Could not reach GitHub");
119121
expect(result.message).toContain("getaddrinfo ENOTFOUND");
120122
}
123+
expect(report).toHaveBeenCalledTimes(1);
124+
expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error);
125+
expect(report.mock.calls[0]?.[1]).toMatchObject({
126+
operation: "exchange_code_for_github_token",
127+
});
128+
report.mockRestore();
121129
});
122130
});

‎packages/connections/src/github-connect.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// still names `credentialPlugin: "http"`).
1313

1414
import { type } from "arktype";
15+
import { reportError } from "@corbits/error-sink";
1516

1617
export const GITHUB_AUTHORIZE_URL = "https://github.com/login/oauth/authorize";
1718
export const GITHUB_TOKEN_EXCHANGE_URL =
@@ -69,6 +70,7 @@ export async function exchangeCodeForGithubToken(
6970
}),
7071
});
7172
} catch (cause) {
73+
reportError(cause, { operation: "exchange_code_for_github_token" });
7274
return {
7375
ok: false,
7476
message:

‎packages/connections/src/gmail-connect.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
// entirely against a stubbed fetch — no Google credentials involved.
33
// Live-key verification against a real Google OAuth app is a deploy
44
// concern (`GMAIL_CLIENT_ID`/`GMAIL_CLIENT_SECRET`), not a test one.
5-
import { expect, test } from "bun:test";
5+
import { expect, spyOn, test } from "bun:test";
6+
import * as errorSink from "@corbits/error-sink";
67

78
import {
89
exchangeCodeForGoogleToken,
@@ -89,3 +90,25 @@ test("a Google error response maps to an honest failure that never echoes token
8990
expect(result.message).toContain("invalid_grant");
9091
expect(result.message).not.toContain("secret-1");
9192
});
93+
94+
test("a transport failure is reported and never crashes", async () => {
95+
const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test");
96+
const result = await exchangeCodeForGoogleToken({
97+
code: "auth-code-1",
98+
redirectUri: "https://bench.example.com/callback",
99+
clientId: "client-1",
100+
clientSecret: "secret-1",
101+
fetchImpl: async () => {
102+
throw new Error("getaddrinfo ENOTFOUND");
103+
},
104+
});
105+
expect(result.ok).toBe(false);
106+
if (result.ok) throw new Error("expected failure");
107+
expect(result.message).toContain("getaddrinfo ENOTFOUND");
108+
expect(report).toHaveBeenCalledTimes(1);
109+
expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error);
110+
expect(report.mock.calls[0]?.[1]).toMatchObject({
111+
operation: "exchange_code_for_google_token",
112+
});
113+
report.mockRestore();
114+
});

‎packages/connections/src/gmail-connect.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
// authorize URL) for the credential row to keep alongside the secret.
1212

1313
import { type } from "arktype";
14+
import { reportError } from "@corbits/error-sink";
1415

1516
export const GOOGLE_AUTHORIZE_URL =
1617
"https://accounts.google.com/o/oauth2/v2/auth";
@@ -83,6 +84,7 @@ export async function exchangeCodeForGoogleToken(
8384
body: params.toString(),
8485
});
8586
} catch (cause) {
87+
reportError(cause, { operation: "exchange_code_for_google_token" });
8688
const message = cause instanceof Error ? cause.message : String(cause);
8789
return { ok: false, message: `Google token exchange failed: ${message}` };
8890
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// The exchange's own contract: parse Hugging Face's response at the trust
2+
// boundary without ever putting token material in a failure message, and
3+
// route a transport failure through reportError.
4+
import { describe, expect, spyOn, test } from "bun:test";
5+
import * as errorSink from "@corbits/error-sink";
6+
import {
7+
exchangeCodeForToken,
8+
type ExchangeFetch,
9+
} from "./huggingface-connect";
10+
11+
describe("exchangeCodeForToken", () => {
12+
test("trades the code and verifier for an access token", async () => {
13+
const fetchImpl: ExchangeFetch = async () =>
14+
new Response(
15+
JSON.stringify({ access_token: "hf_minted_token", expires_in: 3600 }),
16+
{ status: 200 },
17+
);
18+
19+
const result = await exchangeCodeForToken({
20+
code: "auth_code_1",
21+
codeVerifier: "verifier_1",
22+
redirectUri: "https://hub.example.test/callback",
23+
clientId: "client_1",
24+
fetchImpl,
25+
now: () => 0,
26+
});
27+
28+
expect(result).toEqual({
29+
ok: true,
30+
accessToken: "hf_minted_token",
31+
expiresAt: new Date(3600 * 1000).toISOString(),
32+
});
33+
});
34+
35+
test("a transport failure is reported, never token material", async () => {
36+
const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test");
37+
const fetchImpl: ExchangeFetch = async () => {
38+
throw new Error("getaddrinfo ENOTFOUND");
39+
};
40+
41+
const result = await exchangeCodeForToken({
42+
code: "auth_code_1",
43+
codeVerifier: "verifier_1",
44+
redirectUri: "https://hub.example.test/callback",
45+
clientId: "client_1",
46+
fetchImpl,
47+
});
48+
49+
expect(result.ok).toBe(false);
50+
if (!result.ok) {
51+
expect(result.message).toContain("Could not reach Hugging Face");
52+
expect(result.message).toContain("getaddrinfo ENOTFOUND");
53+
}
54+
expect(report).toHaveBeenCalledTimes(1);
55+
expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error);
56+
expect(report.mock.calls[0]?.[1]).toMatchObject({
57+
operation: "exchange_code_for_huggingface_token",
58+
});
59+
report.mockRestore();
60+
});
61+
});

‎packages/connections/src/huggingface-connect.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
// later expiry sweep reads.
1515

1616
import { type } from "arktype";
17+
import { reportError } from "@corbits/error-sink";
1718

1819
export const HUGGINGFACE_AUTHORIZE_URL =
1920
"https://huggingface.co/oauth/authorize";
@@ -90,6 +91,7 @@ export async function exchangeCodeForToken(
9091
body: body.toString(),
9192
});
9293
} catch (cause) {
94+
reportError(cause, { operation: "exchange_code_for_huggingface_token" });
9395
return {
9496
ok: false,
9597
message:

0 commit comments

Comments
 (0)