Skip to content

Commit 2b9debb

Browse files
authored
fix: propagate non-zero exit codes from deploy client (#989)
* fix: handle StaticSitesClient stderr and non-zero exit codes The deploy command silently ignored failures from the StaticSitesClient binary. When the binary crashed (e.g., missing native dependencies on slim Docker images), the CLI reported success and exited with code 0. This fix: - Adds stderr handler to capture binary error output - Adds else branch for non-zero exit codes with diagnostic message - Calls process.exit(1) on failure so CI/CD detects it Fixes #536, #594 Refs: ICM 21000000909499 * test: add tests for StaticSitesClient exit code and stderr handling - stderr is captured and passed to logger.error - Non-zero exit code triggers spinner.fail and error message - process.exit(1) is called on binary failure - Success path (exit code 0) remains unchanged * fix: resolve CI type error in process.exit mock The ReturnType<typeof vi.spyOn> didn't match the 'never' return type of process.exit, causing tsc to fail in CI format check. * fix: skip process.exit(1) in dry-run mode The deploy e2e CI uses --dry-run with a dummy token, which causes StaticSitesClient to fail with BadRequest. In dry-run mode, binary failures should be logged but not cause process.exit(1). Added test: 'should not call process.exit(1) in dry-run mode'
1 parent 59d315d commit 2b9debb

2 files changed

Lines changed: 122 additions & 14 deletions

File tree

src/cli/commands/deploy/deploy.spec.ts

Lines changed: 107 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import "../../../../tests/_mocks/fs.js";
2-
import child_process from "node:child_process";
2+
import child_process, { spawn } from "node:child_process";
3+
import { EventEmitter } from "node:events";
4+
import path from "node:path";
35
import { logger } from "../../../core/utils/logger.js";
46
import { vol } from "memfs";
57
import * as accountModule from "../../../core/account.js";
@@ -10,6 +12,26 @@ import { loadPackageJson } from "../../../core/utils/json.js";
1012

1113
const pkg = loadPackageJson();
1214

15+
// Prevent transitive jsonwebtoken/buffer-equal-constant-time loading error
16+
// by fully mocking modules that pull in Azure SDK → jsonwebtoken chain
17+
vi.mock("../../../core/account.js", () => ({
18+
chooseOrCreateProjectDetails: vi.fn(() => Promise.resolve({ resourceGroup: "mock-rg", staticSiteName: "mock-site" })),
19+
getStaticSiteDeployment: vi.fn(() => Promise.resolve({})),
20+
authenticateWithAzureIdentity: vi.fn(),
21+
listSubscriptions: vi.fn(),
22+
listTenants: vi.fn(),
23+
}));
24+
25+
vi.mock("../login/login.js", () => ({
26+
login: vi.fn(() =>
27+
Promise.resolve({
28+
credentialChain: {},
29+
subscriptionId: "mock-subscription-id",
30+
}),
31+
),
32+
loginCommand: vi.fn(),
33+
}));
34+
1335
vi.mock("../../../core/utils/logger", () => {
1436
return {
1537
logger: {
@@ -22,8 +44,15 @@ vi.mock("../../../core/utils/logger", () => {
2244
};
2345
});
2446

25-
//vi.spyOn(process, "exit").mockImplementation(() => {});
26-
vi.spyOn(child_process, "spawn").mockImplementation(vi.fn());
47+
vi.mock("node:child_process", async (importOriginal) => {
48+
const actual: typeof child_process = await importOriginal();
49+
return {
50+
...actual,
51+
default: { ...actual, spawn: vi.fn() },
52+
spawn: vi.fn(),
53+
};
54+
});
55+
2756
vi.spyOn(deployClientModule, "getDeployClientPath").mockImplementation(() => {
2857
return Promise.resolve({
2958
binary: "mock-binary",
@@ -32,17 +61,6 @@ vi.spyOn(deployClientModule, "getDeployClientPath").mockImplementation(() => {
3261
});
3362
vi.spyOn(deployClientModule, "cleanUp").mockImplementation(() => {});
3463

35-
vi.spyOn(accountModule, "getStaticSiteDeployment").mockImplementation(() => Promise.resolve({}));
36-
37-
vi.spyOn(loginModule, "login").mockImplementation(() => {
38-
return Promise.resolve({
39-
credentialChain: {} as any,
40-
subscriptionId: "mock-subscription-id",
41-
resourceGroup: "mock-resource-group-name",
42-
staticSiteName: "mock-static-site-name",
43-
});
44-
});
45-
4664
describe("deploy", () => {
4765
const OLD_ENV = process.env;
4866

@@ -177,4 +195,79 @@ describe("deploy", () => {
177195
},
178196
});
179197
});
198+
199+
describe("StaticSitesClient process handling", () => {
200+
let mockChild: EventEmitter & { stdout: EventEmitter; stderr: EventEmitter };
201+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
202+
let exitSpy: any;
203+
204+
beforeEach(() => {
205+
// Create mock child process with stdout/stderr EventEmitters
206+
const stdout = new EventEmitter();
207+
const stderr = new EventEmitter();
208+
mockChild = Object.assign(new EventEmitter(), { stdout, stderr });
209+
210+
// Set up spawn mock to return the mock child process
211+
vi.mocked(spawn).mockReturnValue(mockChild as any);
212+
vi.spyOn(deployClientModule, "getDeployClientPath").mockResolvedValue({
213+
binary: "mock-binary",
214+
buildId: "0.0.0",
215+
});
216+
vi.spyOn(deployClientModule, "cleanUp").mockImplementation(() => {});
217+
218+
// Mock process.exit to prevent test runner from exiting
219+
exitSpy = vi.spyOn(process, "exit").mockImplementation((() => undefined) as unknown as () => never);
220+
221+
// Provide deployment token via env to skip login flow
222+
process.env.SWA_CLI_DEPLOYMENT_TOKEN = "test-token";
223+
224+
// Create required filesystem structure in memfs
225+
const cwd = process.cwd();
226+
vol.fromJSON({
227+
[path.join("/test-output", "index.html")]: "hello",
228+
[path.join(cwd, "placeholder")]: "",
229+
});
230+
});
231+
232+
it("should capture stderr and pass to logger.error", async () => {
233+
await deploy({ outputLocation: "/test-output", dryRun: false });
234+
235+
mockChild.stderr.emit("data", Buffer.from("some error from binary"));
236+
237+
expect(logger.error).toHaveBeenCalledWith("some error from binary");
238+
});
239+
240+
it("should fail spinner and log error on non-zero exit code", async () => {
241+
await deploy({ outputLocation: "/test-output", dryRun: false });
242+
243+
mockChild.emit("close", 1);
244+
245+
expect(logger.error).toHaveBeenCalledWith("The deployment binary exited with code 1.");
246+
});
247+
248+
it("should call process.exit(1) on non-zero exit code", async () => {
249+
await deploy({ outputLocation: "/test-output", dryRun: false });
250+
251+
mockChild.emit("close", 127);
252+
253+
expect(exitSpy).toHaveBeenCalledWith(1);
254+
});
255+
256+
it("should succeed without calling process.exit on exit code 0", async () => {
257+
await deploy({ outputLocation: "/test-output", dryRun: false });
258+
259+
mockChild.emit("close", 0);
260+
261+
expect(exitSpy).not.toHaveBeenCalled();
262+
});
263+
264+
it("should not call process.exit(1) on non-zero exit code in dry-run mode", async () => {
265+
await deploy({ outputLocation: "/test-output", dryRun: true });
266+
267+
mockChild.emit("close", 1);
268+
269+
expect(logger.error).toHaveBeenCalledWith("The deployment binary exited with code 1.");
270+
expect(exitSpy).not.toHaveBeenCalled();
271+
});
272+
});
180273
});

src/cli/commands/deploy/deploy.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,13 @@ export async function deploy(options: SWACLIConfig) {
315315
});
316316
});
317317

318+
child.stderr!.on("data", (data) => {
319+
const stderrOutput = data.toString().trim();
320+
if (stderrOutput) {
321+
logger.error(stderrOutput);
322+
}
323+
});
324+
318325
child.on("error", (error) => {
319326
logger.error(error.toString());
320327
});
@@ -325,6 +332,14 @@ export async function deploy(options: SWACLIConfig) {
325332
if (code === 0) {
326333
spinner.succeed(chalk.green(`Project deployed to ${chalk.underline(projectUrl)} 🚀`));
327334
logger.log(``);
335+
} else {
336+
spinner.fail(chalk.red(`Deployment failed with exit code ${code}`));
337+
logger.error(`The deployment binary exited with code ${code}.`);
338+
logger.error(`If you are running in a minimal container image, ensure native dependencies are installed.`);
339+
logger.error(`Run ${chalk.cyan(`ldd ${binary}`)} to check for missing shared libraries.`);
340+
if (!dryRun) {
341+
process.exit(1);
342+
}
328343
}
329344
});
330345
}

0 commit comments

Comments
 (0)