Handling upload errors in logging

packages/alfa-test-utils/src/report/logging.ts

@@ -1,10 +1,11 @@
 import { Array } from "@siteimprove/alfa-array";
 import { Element, Query } from "@siteimprove/alfa-dom";
 import { Equatable } from "@siteimprove/alfa-equatable";
-import { Result } from "@siteimprove/alfa-result";
-import { Sequence } from "@siteimprove/alfa-sequence";
+import { Iterable } from "@siteimprove/alfa-iterable";
 
 import * as json from "@siteimprove/alfa-json";
+import { Err, Result } from "@siteimprove/alfa-result";
+import { Sequence } from "@siteimprove/alfa-sequence";
 import type { Thunk } from "@siteimprove/alfa-thunk";
 import { Page } from "@siteimprove/alfa-web";
 
@@ -18,17 +19,39 @@ import { getRuleTitle } from "./get-rule-title.js";
  *
  * @public
  */
-export class Logging implements Equatable, json.Serializable<Logging.JSON> {
-  public static of(title: string, logs?: Iterable<Logging>): Logging {
-    return new Logging(title, Sequence.from(logs ?? []));
+export class Logging<S extends Logging.Severity = Logging.Severity>
+  implements Equatable, json.Serializable<Logging.JSON>
+{
+  public static of(title: string, logs?: Iterable<Logging>): Logging<"log">;
+
+  public static of<S extends Logging.Severity = "log">(
+    title: string,
+    severity: S,
+    logs?: Iterable<Logging>
+  ): Logging<S>;
+
+  public static of<S extends Logging.Severity = "log">(
+    title: string,
+    severityOrLogs?: S | Iterable<Logging>,
+    logs?: Iterable<Logging>
+  ): Logging<S | "log"> {
+    const innerLogs: Iterable<Logging> =
+      typeof severityOrLogs === "string" ? logs ?? [] : severityOrLogs ?? [];
+
+    const severity =
+      typeof severityOrLogs === "string" ? severityOrLogs : "log";
+
+    return new Logging(title, Sequence.from(innerLogs), severity);
   }
 
   private readonly _title: string;
   private readonly _logs: Sequence<Logging>;
+  private readonly _severity: S;
 
-  protected constructor(title: string, logs: Sequence<Logging>) {
+  protected constructor(title: string, logs: Sequence<Logging>, severity: S) {
     this._title = title;
     this._logs = logs;
+    this._severity = severity;
   }
 
   public get title(): string {
@@ -40,9 +63,13 @@ export class Logging implements Equatable, json.Serializable<Logging.JSON> {
   }
 
   public print(): void {
-    console.group(this._title);
-    this._logs.forEach((log) => log.print());
-    console.groupEnd();
+    if (this._logs.isEmpty()) {
+      console[this._severity](this._title);
+    } else {
+      console.group(this._title);
+      this._logs.forEach((log) => log.print());
+      console.groupEnd();
+    }
   }
 
   public equals(value: Logging): boolean;
@@ -75,6 +102,11 @@ export namespace Logging {
     logs: Sequence.JSON<JSON>;
   }
 
+  /**
+   * {@link https://console.spec.whatwg.org/#loglevel-severity}
+   */
+  export type Severity = "info" | "log" | "warn" | "error";
+
   /** @internal */
   export namespace Defaults {
     export const Title = "Untitled";
@@ -99,6 +131,12 @@ export namespace Logging {
     );
   }
 
+  /**
+   * @internal
+   */
+  export const errorTitle =
+    "The following problem was encountered while uploading results to the Siteimprove Intelligence Platform:";
+
   /**
    * @internal
    */
@@ -108,7 +146,14 @@ export namespace Logging {
     pageReportUrl?: Result<string, string> | string
   ): Logging {
     return Logging.of("Siteimprove found accessibility issues:", [
+      // Show the page title
       Logging.of(chalk.bold(`Page - ${pageTitle ?? Defaults.Title}`)),
+
+      // Show any error during upload: missing or invalid credentials, etc.
+      ...(Err.isErr<string>(pageReportUrl)
+        ? [Logging.of(errorTitle, [Logging.of(pageReportUrl.getErr(), "warn")])]
+        : []),
+
       // "This page contains X issues: URL" (if URL)
       // "This page contains X issues." (otherwise)
       Logging.of(

packages/alfa-test-utils/src/report/sip.ts

@@ -8,8 +8,8 @@ import { Selective } from "@siteimprove/alfa-selective";
 import type { Thunk } from "@siteimprove/alfa-thunk";
 import { Page } from "@siteimprove/alfa-web";
 
-import type { AxiosRequestConfig } from "axios";
-import axios from "axios";
+import type { AxiosRequestConfig, AxiosResponse } from "axios";
+import axios, { AxiosError } from "axios";
 import type { Agent as HttpsAgent } from "https";
 
 import { Audit, type Performance } from "../audit/index.js";
@@ -26,6 +26,15 @@ export namespace SIP {
     export const URL = "https://api.siteimprove.com/v2/a11y/AlfaDevCheck";
     export const Title = "";
     export const Name = undefined;
+
+    export function missingOptions(missing: Array<string>): string {
+      return `The following mandatory option${
+        missing.length === 1 ? " is" : "s are"
+      } missing: ${missing.join(", ")}`;
+    }
+
+    export const badCredentials =
+      "Unauthorized request: the request was made with invalid credentials, verify your username and API key";
   }
 
   /**
@@ -73,27 +82,68 @@ export namespace SIP {
     }
 
     if (missing.length > 0) {
-      return Err.of(
-        `The following mandatory option${
-          missing.length === 1 ? " is" : "s are"
-        } missing: ${missing.join(", ")}`
-      );
+      return Err.of(Defaults.missingOptions(missing));
     }
 
     const config = await Metadata.axiosConfig(audit, options, override);
 
     try {
-      const axiosResponse = await axios.request(config);
+      // The request fail on 4XX and 5XX responses, plus anything that can possibly
+      // go wrong with axios.
+      // We could accept all and handle the errors directly, but since we would
+      // still need a try…catch for the "anything that can possibly go wrong" part,
+      // the benefit would be minimal.
+      const axiosResponse = await axios.request({
+        ...config,
+        // We do not want to throw on 3XX redirections.
+        validateStatus: (status) => status < 400,
+      });
+
       const { pageReportUrl, preSignedUrl, id } = axiosResponse.data;
 
-      await axios.request(S3.axiosConfig(id, preSignedUrl, audit));
+      await axios.request({
+        ...S3.axiosConfig(id, preSignedUrl, audit),
+        // We do not want to throw on 3XX redirections.
+        validateStatus: (status) => status < 400,
+      });
 
       return Ok.of(pageReportUrl);
     } catch (error) {
-      console.error(error);
+      return inspectAxiosError(error);
+    }
+  }
+
+  function inspectAxiosError(error: any): Result<string, string> {
+    if ((error.response ?? undefined) !== undefined) {
+      const { status } = error.response;
+
+      if (status === 401) {
+        // 401 are handled by the generic server, and we don't get custom error message
+        return Err.of(Defaults.badCredentials);
+      }
+
+      if (status >= 400 && status < 500) {
+        // This is a client error, we can get our custom error message
+        return Err.of(
+          `Bad request (${status}): ${error.response.data.details[0].issue}`
+        );
+      }
+
+      if (status >= 500) {
+        // This is a server error, we probably don't have a custom message,
+        // but hopefully axios did the work for us.
+        return Err.of(`Server error (${status}): ${error.message}`);
+      }
+    }
+
+    if (error instanceof AxiosError && error.message !== undefined) {
+      // This is another axios error, we hope they provide meaningful messages.
+      return Err.of(`${error.message}`);
     }
 
-    return Err.of("Could not retrieve a page report URL");
+    // This is something else. It should really not happen since only axios
+    // should have thrown something.
+    return Err.of(`Unexpected error: ${error}`);
   }
 
   /**

packages/alfa-test-utils/test/report/logging.spec.tsx

@@ -25,14 +25,15 @@ const passingRule = makeRule(3, target);
 const audit = makeAudit({
   page: makePage(h.document([target])),
   outcomes: Map.from([
-    [failingRule1.uri, Sequence.from([makeFailed(failingRule2, target)])],
     [
-      failingRule2.uri,
+      failingRule1.uri,
       Sequence.from([
         makeFailed(failingRule1, target),
-        makeFailed(failingRule2, target),
+        makeFailed(failingRule1, target),
+        makeFailed(failingRule1, target),
       ]),
     ],
+    [failingRule2.uri, Sequence.from([makeFailed(failingRule2, target)])],
     [passingRule.uri, Sequence.from([makePassed(passingRule, target)])],
   ]),
   resultAggregates: Map.from([
@@ -84,6 +85,7 @@ test(".fromAggregate() creates a Logging from errored page report URL", (t) => {
     title: "Siteimprove found accessibility issues:",
     logs: [
       { title: chalk.bold(`Page - ${Logging.Defaults.Title}`), logs: [] },
+      { title: Logging.errorTitle, logs: [{ title: "foo", logs: [] }] },
       {
         title: "This page contains 2 issues.",
         logs: [
@@ -183,6 +185,7 @@ test(".fromAudit() creates a Logging from errored page report URL", (t) => {
     title: "Siteimprove found accessibility issues:",
     logs: [
       { title: chalk.bold(`Page - ${Logging.Defaults.Title}`), logs: [] },
+      { title: Logging.errorTitle, logs: [{ title: "foo", logs: [] }] },
       {
         title: "This page contains 2 issues.",
         logs: [
@@ -236,7 +239,7 @@ test(".fromAudit() creates a Logging from correct page report URL", (t) => {
 });
 
 test(".fromAudit() overrides title with a string", (t) => {
-  const actual = Logging.fromAudit(audit, Err.of("foo"), { pageTitle: "foo" });
+  const actual = Logging.fromAudit(audit, undefined, { pageTitle: "foo" });
 
   t.deepEqual(actual.toJSON(), {
     title: "Siteimprove found accessibility issues:",
@@ -254,7 +257,7 @@ test(".fromAudit() overrides title with a string", (t) => {
 });
 
 test(".fromAudit() overrides title with a function", (t) => {
-  const actual = Logging.fromAudit(audit, Err.of("foo"), {
+  const actual = Logging.fromAudit(audit, undefined, {
     pageTitle: (page) =>
       page.document.descendants().find(Text.isText).getUnsafe().data,
   });
@@ -296,7 +299,7 @@ test(".fromAudit() Uses the page title as default", (t) => {
     ]),
   });
 
-  const actual = Logging.fromAudit(audit, Err.of("foo"));
+  const actual = Logging.fromAudit(audit, undefined);
 
   t.deepEqual(actual.toJSON(), {
     title: "Siteimprove found accessibility issues:",