unify interface of http helpers

web/src/client/http.js

@@ -95,21 +95,21 @@ class HTTPClient {
 
   /**
    * @param {string} url - Endpoint URL (e.g., "/l10n/config").
-   * @return {Promise<object>} Server response.
+   * @return {Promise<Response>} Server response.
    */
   async get(url) {
     const response = await fetch(`${this.baseUrl}${url}`, {
       headers: {
         "Content-Type": "application/json",
       },
     });
-    return await response.json();
+    return response;
   }
 
   /**
    * @param {string} url - Endpoint URL (e.g., "/l10n/config").
    * @param {object} data - Data to submit
-   * @return {Promise<object>} Server response.
+   * @return {Promise<Response>} Server response.
    */
   async post(url, data) {
     const response = await fetch(`${this.baseUrl}${url}`, {
@@ -120,18 +120,13 @@ class HTTPClient {
       },
     });
 
-    try {
-      return await response.json();
-    } catch (e) {
-      console.warn("Expecting a JSON response", e);
-      return response.status === 200;
-    }
+    return response;
   }
 
   /**
    * @param {string} url - Endpoint URL (e.g., "/l10n/config").
    * @param {object} data - Data to submit
-   * @return {Promise<object>} Server response.
+   * @return {Promise<Response>} Server response.
    */
   async put(url, data) {
     const response = await fetch(`${this.baseUrl}${url}`, {

web/src/client/l10n.js

@@ -60,15 +60,20 @@ class L10nClient {
    * @return {Promise<String>} Locale ID.
    */
   async getUILocale() {
-    const config = await this.client.get("/l10n/config");
+    const response = await this.client.get("/l10n/config");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return "";
+    }
+    const config = await response.json();
     return config.uiLocale;
   }
 
   /**
    * Sets the locale to translate the installer UI.
    *
    * @param {String} id - Locale ID.
-   * @return {Promise<void>}
+   * @return {Promise<Response>}
    */
   async setUILocale(id) {
     return this.client.patch("/l10n/config", { uiLocale: id });
@@ -82,7 +87,12 @@ class L10nClient {
    * @return {Promise<String>} Keymap ID.
    */
   async getUIKeymap() {
-    const config = await this.client.get("/l10n/config");
+    const response = await this.client.get("/l10n/config");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return "";
+    }
+    const config = await response.json();
     return config.uiKeymap;
   }
 
@@ -92,7 +102,7 @@ class L10nClient {
    * This setting is only relevant in the local installation.
    *
    * @param {String} id - Keymap ID.
-   * @return {Promise<void>}
+   * @return {Promise<Response>}
    */
   async setUIKeymap(id) {
     return this.client.patch("/l10n/config", { uiKeymap: id });
@@ -104,7 +114,12 @@ class L10nClient {
    * @return {Promise<Array<Timezone>>}
    */
   async timezones() {
-    const timezones = await this.client.get("/l10n/timezones");
+    const response = await this.client.get("/l10n/timezones");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return [];
+    }
+    const timezones = await response.json();
     return timezones.map(this.buildTimezone);
   }
 
@@ -136,7 +151,12 @@ class L10nClient {
    * @return {Promise<Array<Locale>>}
    */
   async locales() {
-    const locales = await this.client.get("/l10n/locales");
+    const response = await this.client.get("/l10n/locales");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return [];
+    }
+    const locales = await response.json();
     return locales.map(this.buildLocale);
   }
 
@@ -169,7 +189,12 @@ class L10nClient {
    * @return {Promise<Array<Keymap>>}
    */
   async keymaps() {
-    const keymaps = await this.client.get("/l10n/keymaps");
+    const response = await this.client.get("/l10n/keymaps");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return [];
+    }
+    const keymaps = await response.json();
     return keymaps.map(this.buildKeymap);
   }
 
@@ -242,7 +267,12 @@ class L10nClient {
    * @return {Promise<object>} Localization configuration.
    */
   async getConfig() {
-    return await this.client.get("/l10n/config");
+    const response = await this.client.get("/l10n/config");
+    if (!response.ok) {
+      console.error("Failed to get localization config: ", response);
+      return {};
+    }
+    return await response.json();
   }
 
   /**
@@ -251,7 +281,7 @@ class L10nClient {
    * Convenience method to set l10n the configuration.
    *
    * @param {object} data - Configuration to update. It can just part of the configuration.
-   * @return {Promise<object>}
+   * @return {Promise<Response>}
    */
   async setConfig(data) {
     return this.client.patch("/l10n/config", data);

web/src/client/manager.js

@@ -44,21 +44,21 @@ class ManagerBaseClient {
    *
    * The progress of the probing process can be tracked through installer signals.
    *
-   * @return {Promise<void>}
+   * @return {Promise<Response>}
    */
   startProbing() {
-    return this.client.post("/manager/probe");
+    return this.client.post("/manager/probe", {});
   }
 
   /**
    * Start the installation process
    *
    * The progress of the installation process can be tracked through installer signals.
    *
-   * @return {Promise}
+   * @return {Promise<Response>}
    */
   startInstallation() {
-    return this.client.post("/manager/install");
+    return this.client.post("/manager/install", {});
   }
 
   /**
@@ -70,7 +70,12 @@ class ManagerBaseClient {
    * @return {Promise<boolean>}
    */
   async canInstall() {
-    const installer = await this.client.get("/manager/installer");
+    const response = await this.client.get("/manager/installer");
+    if (!response.ok) {
+      console.error("Failed to get installer config: ", response);
+      return false;
+    }
+    const installer = await response.json();
     return installer.canInstall;
   }
 
@@ -90,7 +95,12 @@ class ManagerBaseClient {
    * @return {Promise<number>}
    */
   async getPhase() {
-    const installer = await this.client.get("/manager/installer");
+    const response = await this.client.get("/manager/installer");
+    if (!response.ok) {
+      console.error("Failed to get installer config: ", response);
+      return 0;
+    }
+    const installer = await response.json();
     return installer.phase;
   }
 
@@ -112,7 +122,7 @@ class ManagerBaseClient {
    * Runs cleanup when installation is done
    */
   finishInstallation() {
-    return this.client.post("/manager/finish");
+    return this.client.post("/manager/finish", {});
   }
 
   /**
@@ -121,7 +131,12 @@ class ManagerBaseClient {
    * @return {Promise<boolean>}
    */
   async useIguana() {
-    const installer = await this.client.get("/manager/installer");
+    const response = await this.client.get("/manager/installer");
+    if (!response.ok) {
+      console.error("Failed to get installer config: ", response);
+      return false;
+    }
+    const installer = await response.json();
     return installer.iguana;
   }
 }

web/src/client/mixins.js

@@ -92,8 +92,14 @@ const WithIssues = (superclass, issues_path, dbus_path) =>
      * @return {Promise<Issue[]>}
      */
     async getIssues() {
-      const issues = await this.client.get(issues_path);
-      return issues.map(buildIssue);
+      const response = await this.client.get(issues_path);
+      if (!response.ok) {
+        console.log("get issues failed with:", response);
+        return [];
+      } else {
+        const issues = await response.json();
+        return issues.map(buildIssue);
+      }
     }
 
     /**
@@ -137,8 +143,14 @@ const WithStatus = (superclass, status_path, service_name) =>
      * @return {Promise<number>} 0 for idle, 1 for busy
      */
     async getStatus() {
-      const status = await this.client.get(status_path);
-      return status.current;
+      const response = await this.client.get(status_path);
+      if (!response.ok) {
+        console.log("get status failed with:", response);
+        return 1; // lets use busy to be on safe side
+      } else {
+        const status = await response.json();
+        return status.current;
+      }
     }
 
     /**
@@ -187,15 +199,24 @@ const WithProgress = (superclass, progress_path, service_name) =>
      *   the current step and whether the service finished or not.
      */
     async getProgress() {
-      const { current_step, max_steps, current_title, finished } = await this.client.get(
-        progress_path,
-      );
-      return {
-        total: max_steps,
-        current: current_step,
-        message: current_title,
-        finished,
-      };
+      const response = await this.client.get(progress_path);
+      if (!response.ok) {
+        console.log("get progress failed with:", response);
+        return {
+          total: 0,
+          current: 0,
+          message: "Failed to get progress",
+          finished: false
+        };
+      } else {
+        const { current_step, max_steps, current_title, finished } = await response.json();
+        return {
+          total: max_steps,
+          current: current_step,
+          message: current_title,
+          finished,
+        };
+      }
     }
 
     /**
@@ -252,15 +273,16 @@ const WithValidation = (superclass, validation_path, service_name) => class exte
    * @return {Promise<ValidationError[]>}
    */
   async getValidationErrors() {
-    let response;
-
-    try {
-      response = await this.client.get(validation_path);
-    } catch (error) {
-      console.error(`Could not get validation errors for ${validation_path}`, error);
+    const response = await this.client.get(validation_path);
+    if (!response.ok) {
+      console.log("get validation failed with:", response);
+      return [{
+        message: "Failed to validate",
+      }];
+    } else {
+      const data = await response.json();
+      return data.errors.map(createError);
     }
-
-    return response.errors.map(createError);
   }
 
   /**

web/src/client/network.test.js

@@ -68,6 +68,15 @@ const mockSettings = {
   networking_enabled: true
 }
 
+const mockJsonFn = jest.fn();
+
+const mockGetFn = jest.fn().mockImplementation(() => {
+  return {
+    ok: true,
+    json: mockJsonFn,
+  };
+});
+
 jest.mock("./http", () => {
   return {
     HTTPClient: jest.fn().mockImplementation(() => {
@@ -78,14 +87,12 @@ jest.mock("./http", () => {
   };
 });
 
-const mockGetFn = jest.fn();
-
 describe("NetworkClient", () => {
   describe("#connections", () => {
     it("returns the list of active connections from the adapter", async () => {
       const http = new HTTPClient(new URL(ADDRESS));
       const client = new NetworkClient(http);
-      mockGetFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]);
+      mockJsonFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]);
       const connections = await client.connections();
       const eth0 = connections.find(c => c.id === "eth0");
       expect(eth0).toEqual(mockConnection);
@@ -96,7 +103,7 @@ describe("NetworkClient", () => {
     it("returns the list of addresses", async () => {
       const http = new HTTPClient(new URL(ADDRESS));
       const client = new NetworkClient(http);
-      mockGetFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]);
+      mockJsonFn.mockResolvedValue([mockWiredConnection, mockWirelessConnection]);
       const addresses = await client.addresses();
       expect(addresses).toEqual([{ address: "192.168.122.100", prefix: 24 }]);
     });
@@ -107,7 +114,7 @@ describe("NetworkClient", () => {
     it("returns network general settings", async () => {
       const http = new HTTPClient(new URL(ADDRESS));
       const client = new NetworkClient(http);
-      mockGetFn.mockResolvedValue(mockSettings);
+      mockJsonFn.mockResolvedValue(mockSettings);
       const settings = await client.settings();
       expect(settings.hostname).toEqual("localhost.localdomain");
     });