Treat Ctrl+C at a y/N prompt as no; clean up temp zips safely; remove dead code

script/command-executor.ts

@@ -461,7 +461,7 @@ function getTotalActiveFromDeploymentMetrics(metrics: DeploymentMetrics): number
   return totalActive;
 }
 
-function initiateExternalAuthenticationAsync(action: string, serverUrl?: string): void {
+function openLoginPage(serverUrl?: string): void {
   const hostname: string = os.hostname();
   const url: string = `${serverUrl || AccountManager.APP_SERVER_URL}/cli-login?hostname=${hostname}`;
   log("Opening your browser...");
@@ -470,7 +470,7 @@ function initiateExternalAuthenticationAsync(action: string, serverUrl?: string)
 }
 
 async function link(command: cli.ILinkCommand): Promise<void> {
-  initiateExternalAuthenticationAsync("link", command.serverUrl);
+  openLoginPage(command.serverUrl);
 }
 
 async function login(command: cli.ILoginCommand): Promise<void> {
@@ -482,12 +482,12 @@ async function login(command: cli.ILoginCommand): Promise<void> {
       throw new Error("Invalid access key.");
     }
   } else {
-    await loginWithExternalAuthentication("login", command.apiServerUrl, command.appServerUrl);
+    await loginWithExternalAuthentication(command.apiServerUrl, command.appServerUrl);
   }
 }
 
-async function loginWithExternalAuthentication(action: string, apiServerUrl?: string, appServerUrl?: string): Promise<void> {
-  initiateExternalAuthenticationAsync(action, appServerUrl);
+async function loginWithExternalAuthentication(apiServerUrl?: string, appServerUrl?: string): Promise<void> {
+  openLoginPage(appServerUrl);
   log(""); // Insert newline
 
   const accessKey: string = await requestAccessKey();
@@ -536,7 +536,7 @@ async function getReactNativeProjectVersionInfo(
 }
 
 async function register(command: cli.IRegisterCommand): Promise<void> {
-  return loginWithExternalAuthentication("register", command.serverUrl);
+  return loginWithExternalAuthentication(command.serverUrl);
 }
 
 async function promote(command: cli.IPromoteCommand): Promise<void> {

script/management-sdk.ts

@@ -364,7 +364,10 @@ class AccountManager {
         })
         .end((err: any, res: superagent.Response) => {
           if (packageFile.isTemporary) {
-            fs.unlinkSync(packageFile.path);
+            // Best effort: failing to delete the temp file must not hide the upload's result
+            try {
+              fs.rmSync(packageFile.path, { force: true });
+            } catch {}
           }
 
           if (err) {
@@ -431,11 +434,16 @@ class AccountManager {
     const files: string[] = await listFiles(filePath);
     const baseDirectoryPath: string = path.dirname(filePath);
     const zipPath: string = path.join(process.cwd(), this.generateRandomFilename(15) + ".zip");
-    await createZip(
-      zipPath,
-      // yazl does not like backslash (\) in the metadata path.
-      files.map((file) => [file, normalizePath(path.relative(baseDirectoryPath, file))])
-    );
+    try {
+      await createZip(
+        zipPath,
+        // yazl does not like backslash (\) in the metadata path.
+        files.map((file) => [file, normalizePath(path.relative(baseDirectoryPath, file))])
+      );
+    } catch (error) {
+      fs.rmSync(zipPath, { force: true });
+      throw error;
+    }
     return { isTemporary: true, path: zipPath };
   }
 

script/types/rest-definitions.ts

@@ -161,5 +161,3 @@ export interface Package extends PackageInfo {
   /*generated*/ size: number;
   /*generated*/ uploadTime: number;
 }
-
-export * from "./rest-definitions";

script/ui/terminal.ts

@@ -24,6 +24,12 @@ export const confirm = (message: string = "Are you sure?"): Promise<boolean> =>
         },
       },
       (err: any, result: any): void => {
+        // Ctrl+C means "no"; other errors (e.g. no terminal) still fail the command
+        if (err?.message === "canceled") {
+          resolve(false);
+          return;
+        }
+
         const accepted = result.response && result.response.toLowerCase() === "y";
         const rejected = !result.response || result.response.toLowerCase() === "n";
 

test/management-sdk.ts

@@ -2,10 +2,15 @@
 // Licensed under the MIT License.
 
 import { strict as assert } from "assert";
+import * as fs from "fs";
+import * as path from "path";
+import * as sinon from "sinon";
 import superagent = require("superagent");
 import installSuperagentMock = require("superagent-mock");
 
 import AccountManager = require("../script/management-sdk");
+import * as fileUtils from "../script/utils/file-utils";
+import { useTempDirectory } from "./helpers/temp-directory";
 
 interface MockedResponse {
   text: string;
@@ -273,4 +278,32 @@ describe("Management SDK", () => {
       assert.equal(await manager.rollback("appName", "deploymentName", "v1"), null);
     });
   });
+
+  describe("uploads", () => {
+    const project = useTempDirectory("management-sdk", { chdir: true });
+
+    afterEach(() => sinon.restore());
+
+    it("release deletes the partly written zip when zipping fails", async () => {
+      project.write("update/index.bundle", "bundle");
+      sinon.stub(fileUtils, "createZip").callsFake(async (zipPath: string) => {
+        fs.writeFileSync(zipPath, "partial");
+        throw new Error("zip failed");
+      });
+
+      await assert.rejects(manager.release("appName", "deploymentName", "update", {}), { message: "zip failed" });
+      assert.deepEqual(
+        fs.readdirSync(project.path).filter((file) => path.extname(file) === ".zip"),
+        []
+      );
+    });
+
+    it("releaseNative rejects with the server error when its zip is already gone", async () => {
+      mockServerResponse(500, "Server error");
+
+      await assert.rejects(manager.releaseNative("appName", "deploymentName", path.join(project.path, "missing.zip"), {}), {
+        statusCode: 500,
+      });
+    });
+  });
 });

test/terminal.ts

@@ -0,0 +1,40 @@
+import { strict as assert } from "assert";
+import * as sinon from "sinon";
+import { confirm } from "../script/ui/terminal";
+
+const prompt = require("prompt");
+
+describe("terminal", () => {
+  afterEach(() => sinon.restore());
+
+  describe("confirm", () => {
+    const answer = (error: Error | null, response?: string) =>
+      sinon
+        .stub(prompt, "get")
+        .callsFake((_schema: unknown, callback: (error: Error | null, result?: unknown) => void) =>
+          callback(error, error ? undefined : { response })
+        );
+
+    beforeEach(() => sinon.stub(prompt, "start"));
+
+    it("accepts y", async () => {
+      answer(null, "y");
+      assert.equal(await confirm(), true);
+    });
+
+    it("rejects an empty answer", async () => {
+      answer(null, "");
+      assert.equal(await confirm(), false);
+    });
+
+    it("rejects when the prompt is cancelled with Ctrl+C", async () => {
+      answer(new Error("canceled"));
+      assert.equal(await confirm(), false);
+    });
+
+    it("still fails on other prompt errors, so scripts don't treat them as a cancel", async () => {
+      answer(new Error("stdin closed"));
+      await assert.rejects(confirm());
+    });
+  });
+});