return Promise<Result> on success; caller can catch errors

mozilla · Aug 2, 2022 · 17d40c3 · 17d40c3
1 parent 70313b7
commit 17d40c3
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 94 deletions.
diff --git a/src/cmd/sign.js b/src/cmd/sign.js
@@ -5,7 +5,6 @@ import {fs} from 'mz';
 import {signAddon as defaultAddonSigner} from 'sign-addon';
 
 import { signAddon as defaultSubmitAddonSigner } from '../util/submit-addon.js';
-import type { SignResult } from '../util/submit-addon.js';
 import defaultBuilder from './build.js';
 import getValidatedManifest, {getManifestId} from '../util/manifest.js';
 import {withTempDir} from '../util/temp-dir.js';
@@ -14,8 +13,6 @@ import {prepareArtifactsDir} from '../util/artifacts.js';
 import {createLogger} from '../util/logger.js';
 import type {ExtensionManifest} from '../util/manifest.js';
 
-export type { SignResult };
-
 const log = createLogger(import.meta.url);
 
 const defaultAsyncFsReadFile: (string) => Promise<Buffer> =
@@ -49,6 +46,12 @@ export type SignOptions = {
   shouldExitProgram?: boolean,
 };
 
+export type SignResult = {|
+  success: true,
+  id: string,
+  downloadedFiles: Array<string>,
+|};
+
 export default function sign(
   {
     apiHost,
@@ -128,7 +131,23 @@ export default function sign(
         log.warn('No extension ID specified (it will be auto-generated)');
       }
 
-      let signingResult;
+      if (useSubmissionApi && !channel) {
+        throw new UsageError(
+          'channel is a required parameter for the addon submission API'
+        );
+      }
+
+      if (useSubmissionApi && apiProxy) {
+        // https://github.com/mozilla/web-ext/issues/2472
+        throw new UsageError(
+          "apiProxy isn't yet supported for the addon submission API"
+        );
+      }
+
+      if (useSubmissionApi && apiHost.endsWith('/')) {
+        apiHost = apiHost.slice(0, -1);
+      }
+
       const signSubmitArgs = {
         apiKey,
         apiSecret,
@@ -138,49 +157,38 @@ export default function sign(
         downloadDir: artifactsDir,
         channel,
       };
-      if (useSubmissionApi) {
-        if (!channel) {
-          throw new UsageError(
-            'channel is a required parameter for the addon submission API'
-          );
-        }
-        if (apiProxy) {
-          // https://github.com/mozilla/web-ext/issues/2472
-          throw new UsageError(
-            "apiProxy isn't yet supported for the addon submission API"
-          );
-        }
-        if (apiHost.endsWith('/')) {
-          apiHost = apiHost.slice(0, -1);
-        }
-        signingResult = await submitAddon(
-          {...signSubmitArgs, apiHost, channel}
-        );
-      } else {
-        signingResult = await signAddon({
-          ...signSubmitArgs,
-          apiUrlPrefix,
-          apiProxy,
-          verbose,
-          version: manifestData.version,
-        });
-      }
 
-      // All information about the downloaded files would have
-      // already been logged by signAddon().
-      if (signingResult.success) {
-        await saveIdToSourceDir(sourceDir, signingResult.id);
-        log.info(`Extension ID: ${signingResult.id}`);
-        log.info('SUCCESS');
-      } else {
+      let result;
+      try {
+        if (useSubmissionApi && channel) {
+          // we verify `channel` is set above, this is just for Flow
+          result = await submitAddon({...signSubmitArgs, apiHost, channel});
+        } else {
+          const { success, ...signAddonResult } = await signAddon({
+            ...signSubmitArgs,
+            apiUrlPrefix,
+            apiProxy,
+            verbose,
+            version: manifestData.version,
+          });
+          if (!success) {
+            throw new Error();
+          }
+          result = signAddonResult;
+        }
+      } catch (clientError) {
         log.info('FAIL');
         throw new WebExtError(
-          'The extension could not be signed');
+          clientError.message || 'The extension could not be signed');
       }
 
-      return signingResult;
-    }
-  );
+      // All information about the downloaded files would have
+      // already been logged by signAddon().
+      await saveIdToSourceDir(sourceDir, result.id);
+      log.info(`Extension ID: ${result.id}`);
+      log.info('SUCCESS');
+      return {...result, success: true};
+    });
 }
 
 

diff --git a/src/util/submit-addon.js b/src/util/submit-addon.js
@@ -11,17 +11,10 @@ import {createLogger} from './../util/logger.js';
 
 const log = createLogger(import.meta.url);
 
-type SignResultSuccess = {|
-  success: true,
+export type Result = {|
   id: string,
   downloadedFiles: Array<string>,
 |};
-type SignResultFailure = {|
-  success: false,
-  id: null,
-  downloadedFiles: Array<string>,
-|};
-export type SignResult = SignResultSuccess | SignResultFailure;
 
 type ClientConstructorParams = {|
   apiKey: string,
@@ -234,7 +227,7 @@ export default class Client {
   async downloadSignedFile(
     fileUrl: string,
     addonId: string
-  ): Promise<SignResult> {
+  ): Promise<Result> {
     const filename = fileUrl.split('/').pop(); // get the name from fileUrl
     const dest = `${this.downloadDir}/${filename}`;
     const response = await this.fetch(fileUrl);
@@ -244,7 +237,6 @@ export default class Client {
     }
     await this.saveToFile(response.body, dest);
     return {
-      success: true,
       id: addonId,
       downloadedFiles: [filename],
     };
@@ -259,7 +251,7 @@ export default class Client {
     xpiPath: string,
     channel: string,
     metaDataJson: Object
-  ): Promise<SignResult> {
+  ): Promise<Result> {
     const uploadUuid = await this.doUploadSubmit(xpiPath, channel);
 
     const versionObject =
@@ -277,7 +269,7 @@ export default class Client {
     channel: string,
     addonId: string,
     metaDataJson: Object
-  ): Promise<SignResult> {
+  ): Promise<Result> {
     const upload_uuid = await this.doUploadSubmit(xpiPath, channel);
 
     await this.doNewAddonOrVersionSubmit(addonId, upload_uuid, metaDataJson);
@@ -315,7 +307,7 @@ export async function signAddon({
   xpiPath,
   downloadDir,
   channel,
-}: signAddonParams): Promise<SignResult> {
+}: signAddonParams): Promise<Result> {
   try {
     const stats = await fsPromises.stat(xpiPath);
 
@@ -336,26 +328,9 @@ export async function signAddon({
     downloadDir,
   });
 
-  try {
-    if (id === undefined) {
-      return client.postNewAddon(
-        xpiPath,
-        channel,
-        {},
-      );
-    } else {
-      return client.putVersion(
-        xpiPath,
-        channel,
-        id,
-        {},
-      );
-    }
-  } catch (clientError) {
-    return Promise.resolve({
-      success: false,
-      id: null,
-      downloadedFiles: [],
-    });
+  if (id === undefined) {
+    return client.postNewAddon(xpiPath, channel, {});
+  } else {
+    return client.putVersion(xpiPath, channel, id, {});
   }
 }
diff --git a/tests/unit/test-cmd/test.sign.js b/tests/unit/test-cmd/test.sign.js
@@ -499,6 +499,21 @@ describe('sign', () => {
     }
   ));
 
+  it('passes through a signing exception from submitAddon', () => withTempDir(
+    (tmpDir) => {
+      const stubs = getStubs();
+      stubs.submitAddon = () => Promise.reject(new Error('some signing error'));
+
+      return sign(tmpDir, stubs, {
+        extraArgs: { useSubmissionApi: true, channel: 'listed' },
+      })
+        .then(makeSureItFails())
+        .catch((error) => {
+          assert.match(error.message, /signing error/);
+        });
+    }
+  ));
+
   describe('saveIdToSourceDir', () => {
 
     it('saves an extension ID to file', () => withTempDir(

diff --git a/tests/unit/test-util/test.submit-addon.js b/tests/unit/test-util/test.submit-addon.js
@@ -100,23 +100,6 @@ describe('util.submit-addon', () => {
           );
         });
     });
-
-    it('catch errors and returns a SignResult promise', async () => {
-      const xpiPath = 'this/path/xpi.xpi';
-      const channel = 'thisChannel';
-      postNewAddonStub.throws();
-
-      const result = await signAddon({
-        ...signAddonDefaults,
-        xpiPath,
-        channel,
-      });
-      expect(result).to.eql({
-        success: false,
-        id: null,
-        downloadedFiles: [],
-      });
-    });
   });
 
   describe('Client', () => {
@@ -354,7 +337,6 @@ describe('util.submit-addon', () => {
 
         const result = await client.downloadSignedFile(fileUrl, addonId);
         expect(result).to.eql({
-          success: true,
           id: addonId,
           downloadedFiles: [filename],
         });