Skip to content

Commit 86b1c96

Browse files
committed
fix(sdk,pulumi): bound provider API calls and stop a failed refresh failing the update (#1308)
Synced from sferarc/pgbeam@2d6e1ee
1 parent e85dcb6 commit 86b1c96

13 files changed

Lines changed: 577 additions & 44 deletions

src/agentCredential.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { AgentCredential as AgentCredentialData } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
export interface AgentCredentialArgs {
@@ -106,6 +112,11 @@ const agentCredentialProvider: pulumi.dynamic.ResourceProvider = {
106112
}),
107113
};
108114
} catch (err) {
115+
if (isApiUnreachable(err)) {
116+
warnRefreshSkipped("AgentCredential", id, err);
117+
return { id, props };
118+
}
119+
109120
if (apiErrorStatus(err) === 404 && props.name) {
110121
const list = (await api.agents.listAgentCredentials({
111122
pathParams: { project_id: String(props.projectId) },

src/cacheRule.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { CacheRuleEntry } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
export interface CacheRuleArgs {
@@ -110,6 +116,11 @@ const cacheRuleProvider: pulumi.dynamic.ResourceProvider = {
110116
}),
111117
};
112118
} catch (err) {
119+
if (isApiUnreachable(err)) {
120+
warnRefreshSkipped("CacheRule", id, err);
121+
return { id, props };
122+
}
123+
113124
handleApiError("read", "CacheRule", err);
114125
}
115126
},

src/customDomain.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { CustomDomain as CustomDomainData } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
interface DnsInstructions {
@@ -104,6 +110,11 @@ const customDomainProvider: pulumi.dynamic.ResourceProvider = {
104110
}),
105111
};
106112
} catch (err) {
113+
if (isApiUnreachable(err)) {
114+
warnRefreshSkipped("CustomDomain", id, err);
115+
return { id, props };
116+
}
117+
107118
handleApiError("read", "CustomDomain", err);
108119
}
109120
},

src/database.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { Database as DatabaseData } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
interface CacheConfig {
@@ -178,6 +184,11 @@ const databaseProvider: pulumi.dynamic.ResourceProvider = {
178184
}),
179185
};
180186
} catch (err) {
187+
if (isApiUnreachable(err)) {
188+
warnRefreshSkipped("Database", id, err);
189+
return { id, props };
190+
}
191+
181192
handleApiError("read", "Database", err);
182193
}
183194
},

src/policyProfile.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { MaskingRule, PolicyProfile as PolicyProfileData, RowFilter } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
interface StatementRules {
@@ -208,6 +214,11 @@ const policyProfileProvider: pulumi.dynamic.ResourceProvider = {
208214
}),
209215
};
210216
} catch (err) {
217+
if (isApiUnreachable(err)) {
218+
warnRefreshSkipped("PolicyProfile", id, err);
219+
return { id, props };
220+
}
221+
211222
if (apiErrorStatus(err) === 404 && props.name) {
212223
const list = (await api.policies.listPolicyProfiles({
213224
pathParams: { project_id: String(props.projectId) },

src/project.gen.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22

33
import * as pulumi from "@pulumi/pulumi";
44
import type { CidrEntry, Project as ProjectData } from "pgbeam";
5-
import { apiErrorStatus, createClient, handleApiError } from "./provider.js";
5+
import {
6+
apiErrorStatus,
7+
createClient,
8+
handleApiError,
9+
isApiUnreachable,
10+
warnRefreshSkipped,
11+
} from "./provider.js";
612
import { stripUndefined } from "./utils.js";
713

814
export interface CidrEntryArgs {
@@ -249,6 +255,11 @@ const projectProvider: pulumi.dynamic.ResourceProvider = {
249255
}),
250256
};
251257
} catch (err) {
258+
if (isApiUnreachable(err)) {
259+
warnRefreshSkipped("Project", id, err);
260+
return { id, props };
261+
}
262+
252263
if (apiErrorStatus(err) === 404 && props.name) {
253264
const orgId = String(props.orgId);
254265
const list = (await api.projects.listProjects({

src/provider.test.ts

Lines changed: 153 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,37 @@ vi.mock("@pulumi/pulumi", () => ({
99
})),
1010
}));
1111

12-
// Mock the PgBeam SDK
13-
vi.mock("pgbeam", () => {
14-
class MockApiError extends Error {
15-
status: number;
16-
statusText: string;
17-
body: unknown;
18-
constructor(status: number, statusText: string, body?: unknown) {
19-
super(`${status} ${statusText}`);
20-
this.name = "ApiError";
21-
this.status = status;
22-
this.statusText = statusText;
23-
this.body = body;
24-
}
25-
}
12+
// Record the options the provider hands to the SDK client, so the retry and
13+
// timeout policy can be asserted without a network call.
14+
const clientOptions = vi.hoisted(() => ({
15+
last: undefined as Record<string, unknown> | undefined,
16+
}));
17+
18+
// Mock only the client transport. ApiError and describeError stay real so the
19+
// tests exercise the same error shapes the SDK actually throws.
20+
vi.mock("pgbeam", async (importOriginal) => {
21+
const actual = await importOriginal<typeof import("pgbeam")>();
2622

2723
class MockPgBeamClient {
2824
api: Record<string, unknown>;
29-
constructor(_opts: Record<string, unknown>) {
25+
constructor(opts: Record<string, unknown>) {
26+
clientOptions.last = opts;
3027
this.api = { projects: {}, databases: {}, platform: {} };
3128
}
3229
}
3330

34-
return {
35-
PgBeamClient: MockPgBeamClient,
36-
ApiError: MockApiError,
37-
};
31+
return { ...actual, PgBeamClient: MockPgBeamClient };
3832
});
3933

40-
import { ApiError } from "pgbeam";
41-
import { apiErrorStatus, configure, createClient, handleApiError } from "./provider";
34+
import { ApiError, NetworkError } from "pgbeam";
35+
import {
36+
apiErrorStatus,
37+
configure,
38+
createClient,
39+
handleApiError,
40+
isApiUnreachable,
41+
warnRefreshSkipped,
42+
} from "./provider";
4243

4344
// ---------------------------------------------------------------------------
4445
// configure
@@ -73,6 +74,27 @@ describe("createClient", () => {
7374
const client = createClient();
7475
expect(client).toBeDefined();
7576
});
77+
78+
it("bounds every request with a timeout", () => {
79+
configure({ apiKey: "test-key" });
80+
createClient();
81+
82+
expect(clientOptions.last?.timeoutMs).toBe(15_000);
83+
});
84+
85+
it("bounds the retry ladder with a total budget", () => {
86+
configure({ apiKey: "test-key" });
87+
createClient();
88+
89+
const retry = clientOptions.last?.retry as Record<string, number>;
90+
expect(retry.totalBudgetMs).toBe(60_000);
91+
// Worst case is the budget plus one final request timeout, well under the
92+
// five minutes an unbounded ladder used to spend before failing.
93+
const maxBackoff = Array.from({ length: retry.maxRetries }, (_, i) =>
94+
Math.min(retry.initialDelayMs * 2 ** i, retry.maxDelayMs),
95+
).reduce((a, b) => a + b, 0);
96+
expect(maxBackoff).toBeLessThanOrEqual(retry.totalBudgetMs);
97+
});
7698
});
7799

78100
// ---------------------------------------------------------------------------
@@ -98,16 +120,123 @@ describe("handleApiError", () => {
98120
);
99121
});
100122

101-
it("re-throws non-ApiError errors as-is", () => {
123+
it("names the operation and resource for a non-ApiError", () => {
102124
const err = new Error("network failure");
103-
expect(() => handleApiError("delete", "Replica", err)).toThrow("network failure");
125+
expect(() => handleApiError("delete", "Replica", err)).toThrow(
126+
"PgBeam delete Replica failed: Error: network failure",
127+
);
128+
});
129+
130+
it("unwraps the cause chain hidden behind 'fetch failed'", () => {
131+
const cause = Object.assign(new Error("connect ECONNREFUSED 10.0.0.1:443"), {
132+
code: "ECONNREFUSED",
133+
});
134+
const err = new TypeError("fetch failed", { cause });
135+
136+
expect(() => handleApiError("read", "Project", err)).toThrow(
137+
/PgBeam read Project failed: TypeError: fetch failed \(caused by Error: connect ECONNREFUSED 10\.0\.0\.1:443 \[ECONNREFUSED\]\)/,
138+
);
104139
});
105140

106-
it("re-throws non-Error values", () => {
141+
it("keeps the original error as the cause", () => {
142+
const err = new Error("boom");
143+
try {
144+
handleApiError("update", "Project", err);
145+
expect.unreachable("handleApiError must throw");
146+
} catch (thrown) {
147+
expect((thrown as Error).cause).toBe(err);
148+
}
149+
});
150+
151+
it("wraps non-Error values", () => {
107152
expect(() => handleApiError("create", "Project", "string error")).toThrow("string error");
108153
});
109154
});
110155

156+
// ---------------------------------------------------------------------------
157+
// isApiUnreachable
158+
// ---------------------------------------------------------------------------
159+
describe("isApiUnreachable", () => {
160+
it("is true for a NetworkError from the SDK", () => {
161+
const err = new NetworkError({
162+
method: "GET",
163+
url: "https://api.staging.pgbeam.dev/v1/projects/p1",
164+
attempts: 5,
165+
elapsedMs: 1234,
166+
timedOut: false,
167+
timeoutMs: 15_000,
168+
cause: new TypeError("fetch failed"),
169+
});
170+
expect(isApiUnreachable(err)).toBe(true);
171+
});
172+
173+
it("is true for a bare undici 'fetch failed'", () => {
174+
expect(isApiUnreachable(new TypeError("fetch failed"))).toBe(true);
175+
});
176+
177+
it("is true for a refused connection nested in the cause chain", () => {
178+
const cause = Object.assign(new Error("connect ECONNREFUSED 10.0.0.1:443"), {
179+
code: "ECONNREFUSED",
180+
});
181+
expect(isApiUnreachable(new Error("wrapped", { cause }))).toBe(true);
182+
});
183+
184+
it("is true for an aborted or timed-out request", () => {
185+
const timeout = Object.assign(new Error("aborted"), { name: "TimeoutError" });
186+
expect(isApiUnreachable(timeout)).toBe(true);
187+
});
188+
189+
it("is true for a gateway status, which is not an answer from the API", () => {
190+
expect(isApiUnreachable(new ApiError(502, "Bad Gateway", null))).toBe(true);
191+
expect(isApiUnreachable(new ApiError(503, "Service Unavailable", null))).toBe(true);
192+
expect(isApiUnreachable(new ApiError(504, "Gateway Timeout", null))).toBe(true);
193+
});
194+
195+
it("is false when the API gave a considered answer", () => {
196+
expect(isApiUnreachable(new ApiError(404, "Not Found", null))).toBe(false);
197+
expect(isApiUnreachable(new ApiError(401, "Unauthorized", null))).toBe(false);
198+
expect(isApiUnreachable(new ApiError(422, "Unprocessable", null))).toBe(false);
199+
// A 500 means the API itself answered and something is genuinely broken.
200+
expect(isApiUnreachable(new ApiError(500, "Internal Server Error", null))).toBe(false);
201+
});
202+
203+
it("is false for an ordinary error", () => {
204+
expect(isApiUnreachable(new Error("Project prj_1 not found"))).toBe(false);
205+
expect(isApiUnreachable(null)).toBe(false);
206+
expect(isApiUnreachable("boom")).toBe(false);
207+
});
208+
209+
it("does not follow a cause chain forever", () => {
210+
const err: Error & { cause?: unknown } = new Error("loop");
211+
err.cause = err;
212+
expect(isApiUnreachable(err)).toBe(false);
213+
});
214+
});
215+
216+
// ---------------------------------------------------------------------------
217+
// warnRefreshSkipped
218+
// ---------------------------------------------------------------------------
219+
describe("warnRefreshSkipped", () => {
220+
it("says which resource was skipped and why", () => {
221+
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
222+
try {
223+
const cause = Object.assign(new Error("connect ECONNREFUSED 10.0.0.1:443"), {
224+
code: "ECONNREFUSED",
225+
});
226+
warnRefreshSkipped("Project", "prj_1", new TypeError("fetch failed", { cause }));
227+
228+
expect(warn).toHaveBeenCalledOnce();
229+
const message = warn.mock.calls[0][0] as string;
230+
expect(message).toContain("Project");
231+
expect(message).toContain("prj_1");
232+
expect(message).toContain("ECONNREFUSED");
233+
expect(message).toContain("Keeping the last known state");
234+
} finally {
235+
warn.mockRestore();
236+
}
237+
});
238+
});
239+
111240
// ---------------------------------------------------------------------------
112241
// apiErrorStatus
113242
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)