Bug 1634323 - Document and catch server error on attachment download r=gbeckley

Differential Revision: https://phabricator.services.mozilla.com/D142059

services/settings/Attachments.jsm

@@ -29,6 +29,14 @@ class BadContentError extends Error {
   }
 }
 
+class ServerInfoError extends Error {
+  constructor(error) {
+    super(`Server response is invalid ${error}`);
+    this.name = "ServerInfoError";
+    this.original = error;
+  }
+}
+
 // Helper for the `download` method for commonly used methods, to help with
 // lazily accessing the record and attachment content.
 class LazyRecordAndBuffer {
@@ -92,6 +100,9 @@ class Downloader {
   static get BadContentError() {
     return BadContentError;
   }
+  static get ServerInfoError() {
+    return ServerInfoError;
+  }
 
   constructor(...folders) {
     this.folders = ["settings", ...folders];
@@ -128,6 +139,8 @@ class Downloader {
    *                                         (default: `false`)
    * @throws {Downloader.DownloadError} if the file could not be fetched.
    * @throws {Downloader.BadContentError} if the downloaded content integrity is not valid.
+   * @throws {Downloader.ServerInfoError} if the server response is not valid.
+   * @throws {NetworkError} if fetching the server infos and fetching the attachment fails.
    * @returns {Object} An object with two properties:
    *   `buffer` `ArrayBuffer`: the file content.
    *   `record` `Object`: record associated with the attachment.
@@ -282,6 +295,8 @@ class Downloader {
    * @param {Number} options.retries Number of times download should be retried (default: `3`)
    * @throws {Downloader.DownloadError} if the file could not be fetched.
    * @throws {Downloader.BadContentError} if the downloaded file integrity is not valid.
+   * @throws {Downloader.ServerInfoError} if the server response is not valid.
+   * @throws {NetworkError} if fetching the attachment fails.
    * @returns {String} the absolute file path to the downloaded attachment.
    */
   async downloadToDisk(record, options = {}) {
@@ -396,8 +411,13 @@ class Downloader {
 
   async _baseAttachmentsURL() {
     if (!this._cdnURL) {
-      const server = Utils.SERVER_URL;
-      const serverInfo = await (await Utils.fetch(`${server}/`)).json();
+      const resp = await Utils.fetch(`${Utils.SERVER_URL}/`);
+      let serverInfo;
+      try {
+        serverInfo = await resp.json();
+      } catch (error) {
+        throw new Downloader.ServerInfoError(error);
+      }
       // Server capabilities expose attachments configuration.
       const {
         capabilities: {

services/settings/test/unit/test_attachments_downloader.js

@@ -61,20 +61,6 @@ function run_test() {
     do_get_file("test_attachments_downloader")
   );
 
-  server.registerPathHandler("/v1/", (request, response) => {
-    response.write(
-      JSON.stringify({
-        capabilities: {
-          attachments: {
-            base_url: `http://localhost:${server.identity.primaryPort}/cdn/`,
-          },
-        },
-      })
-    );
-    response.setHeader("Content-Type", "application/json; charset=UTF-8");
-    response.setStatusLine(null, 200, "OK");
-  });
-
   Services.prefs.setCharPref(
     "services.settings.server",
     `http://localhost:${server.identity.primaryPort}/v1`
@@ -97,12 +83,45 @@ async function clear_state() {
     // Writable to allow specific tests to override cacheImpl.
     writable: true,
   });
-
   await downloader.deleteDownloaded(RECORD);
+
+  server.registerPathHandler("/v1/", (request, response) => {
+    response.write(
+      JSON.stringify({
+        capabilities: {
+          attachments: {
+            base_url: `http://localhost:${server.identity.primaryPort}/cdn/`,
+          },
+        },
+      })
+    );
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
+    response.setStatusLine(null, 200, "OK");
+  });
 }
 
 add_task(clear_state);
 
+add_task(
+  async function test_download_throws_server_info_error_if_invalid_response() {
+    server.registerPathHandler("/v1/", (request, response) => {
+      response.write("{bad json content");
+      response.setHeader("Content-Type", "application/json; charset=UTF-8");
+      response.setStatusLine(null, 200, "OK");
+    });
+
+    let error;
+    try {
+      await downloader.download(RECORD);
+    } catch (e) {
+      error = e;
+    }
+
+    Assert.ok(error instanceof Downloader.ServerInfoError);
+  }
+);
+add_task(clear_state);
+
 add_task(async function test_download_writes_file_in_profile() {
   const fileURL = await downloader.downloadToDisk(RECORD);
   const localFilePath = pathFromURL(fileURL);