Integrate problem details into Solid client library (#2458)

--------- Co-authored-by: Nicolas Ayral Seydoux <[email protected]> Co-authored-by: Jarlath Holleran <[email protected]>
inrupt · Aug 20, 2024 · c9bfbcc · c9bfbcc
1 parent be84e6a
commit c9bfbcc
Show file tree

Hide file tree

Showing 8 changed files with 193 additions and 33 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -201,6 +201,7 @@
     "webpack-cli": "^5.1.4"
   },
   "dependencies": {
+    "@inrupt/solid-client-errors": "^0.0.1",
     "@rdfjs/dataset": "^1.1.1",
     "buffer": "^6.0.3",
     "http-link-header": "^1.1.1",

diff --git a/src/resource/file.test.ts b/src/resource/file.test.ts
@@ -27,6 +27,8 @@ import {
   beforeEach,
   afterEach,
 } from "@jest/globals";
+import { DEFAULT_TYPE } from "@inrupt/solid-client-errors";
+import { FetchError } from "./resource";
 import {
   getFile,
   deleteFile,
@@ -35,6 +37,7 @@ import {
   flattenHeaders,
 } from "./file";
 import type { WithResourceInfo } from "../interfaces";
+import { mockResponse } from "../tests.internal";
 
 describe("flattenHeaders", () => {
   it("returns an empty object for undefined headers", () => {
@@ -244,6 +247,112 @@ describe("getFile", () => {
       message: expect.stringMatching("Teapots don't make coffee"),
     });
   });
+
+  it("throws an instance of FetchError when a request failed", async () => {
+    const mockFetch = jest.fn<typeof fetch>().mockResolvedValue(
+      new Response("I'm a teapot!", {
+        status: 418,
+        statusText: "I'm a teapot!",
+      }),
+    );
+
+    const fetchPromise = getFile("https://arbitrary.pod/resource", {
+      fetch: mockFetch,
+    });
+
+    const error: FetchError = await fetchPromise.catch((err) => err);
+
+    expect(error).toBeInstanceOf(FetchError);
+
+    // Verify the problem details data
+    expect(error.problemDetails.type).toBe(DEFAULT_TYPE);
+    expect(error.problemDetails.title).toBe("I'm a teapot!");
+    expect(error.problemDetails.status).toBe(418);
+    expect(error.problemDetails.detail).toBeUndefined();
+    expect(error.problemDetails.instance).toBeUndefined();
+  });
+
+  it("throws an instance of FetchError when a request failed with problem details", async () => {
+    const url = "https://arbitrary.pod/resource";
+    const problem = {
+      type: new URL("https://error.test/NotFound"),
+      title: "Not Found",
+      detail: "No resource was found at this location",
+      status: 404,
+      instance: new URL(url),
+    };
+    const mockFetch = jest.fn<typeof fetch>().mockResolvedValue(
+      mockResponse(
+        JSON.stringify(problem),
+        {
+          status: 404,
+          statusText: "Not Found",
+          headers: {
+            "Content-Type": "application/problem+json",
+          },
+        },
+        url,
+      ),
+    );
+
+    const fetchPromise = getFile(url, {
+      fetch: mockFetch,
+    });
+
+    const error: FetchError = await fetchPromise.catch((err) => err);
+
+    expect(error).toBeInstanceOf(FetchError);
+
+    // Verify the problem details data
+    expect(error.problemDetails.type).toEqual(problem.type);
+    expect(error.problemDetails.title).toBe(problem.title);
+    expect(error.problemDetails.status).toBe(problem.status);
+    expect(error.problemDetails.detail).toBe(problem.detail);
+    expect(error.problemDetails.instance).toEqual(problem.instance);
+  });
+
+  it("throws an instance of FetchError when a request failed with problem details using relative URIs", async () => {
+    const url = "https://arbitrary.pod/container/resource";
+    const problem = {
+      type: "/errors/NotFound",
+      title: "Not Found",
+      detail: "No resource was found at this location",
+      status: 404,
+      instance: "relative-url",
+    };
+    const mockFetch = jest.fn<typeof fetch>().mockResolvedValue(
+      mockResponse(
+        JSON.stringify(problem),
+        {
+          status: 404,
+          statusText: "Not Found",
+          headers: {
+            "Content-Type": "application/problem+json",
+          },
+        },
+        url,
+      ),
+    );
+
+    const fetchPromise = getFile(url, {
+      fetch: mockFetch,
+    });
+
+    const error: FetchError = await fetchPromise.catch((err) => err);
+
+    expect(error).toBeInstanceOf(FetchError);
+
+    // Verify the problem details data
+    expect(error.problemDetails.type).toEqual(
+      new URL("https://arbitrary.pod/errors/NotFound"),
+    );
+    expect(error.problemDetails.title).toBe(problem.title);
+    expect(error.problemDetails.status).toBe(problem.status);
+    expect(error.problemDetails.detail).toBe(problem.detail);
+    expect(error.problemDetails.instance).toEqual(
+      new URL("https://arbitrary.pod/container/relative-url"),
+    );
+  });
 });
 
 describe("Non-RDF data deletion", () => {

diff --git a/src/resource/file.ts b/src/resource/file.ts
@@ -89,11 +89,13 @@ export async function getFile(
     options?.init,
   );
   if (internal_isUnsuccessfulResponse(response)) {
+    const errorBody = await response.clone().text();
     throw new FetchError(
       `Fetching the File failed: [${response.status}] [${
         response.statusText
-      }] ${await response.text()}.`,
+      }] ${errorBody}.`,
       response,
+      errorBody,
     );
   }
   const resourceInfo = internal_parseResourceInfo(response);
@@ -135,11 +137,13 @@ export async function deleteFile(
   });
 
   if (internal_isUnsuccessfulResponse(response)) {
+    const errorBody = await response.clone().text();
     throw new FetchError(
       `Deleting the file at [${url}] failed: [${response.status}] [${
         response.statusText
-      }] ${await response.text()}.`,
+      }] ${errorBody}.`,
       response,
+      errorBody,
     );
   }
 }
@@ -219,11 +223,13 @@ export async function saveFileInContainer<
   const response = await writeFile(folderUrlString, file, "POST", options);
 
   if (internal_isUnsuccessfulResponse(response)) {
+    const errorBody = await response.clone().text();
     throw new FetchError(
       `Saving the file in [${folderUrl}] failed: [${response.status}] [${
         response.statusText
-      }] ${await response.text()}.`,
+      }] ${errorBody}.`,
       response,
+      errorBody,
     );
   }
 
@@ -306,11 +312,13 @@ export async function overwriteFile<FileExt extends File | BlobFile | NodeFile>(
   const response = await writeFile(fileUrlString, file, "PUT", options);
 
   if (internal_isUnsuccessfulResponse(response)) {
+    const errorBody = await response.clone().text();
     throw new FetchError(
       `Overwriting the file at [${fileUrlString}] failed: [${
         response.status
-      }] [${response.statusText}] ${await response.text()}.`,
+      }] [${response.statusText}] ${errorBody}.`,
       response,
+      errorBody,
     );
   }
 

diff --git a/src/resource/resource.test.ts b/src/resource/resource.test.ts
@@ -25,7 +25,6 @@ import {
   getSourceIri,
   getPodOwner,
   isPodOwner,
-  FetchError,
   isContainer,
   isRawData,
   getContentType,
@@ -474,21 +473,6 @@ describe("getResourceInfo", () => {
 
     await expect(fetchPromise).rejects.toBeInstanceOf(SolidClientError);
   });
-
-  it("throws an instance of FetchError when a request failed", async () => {
-    const mockFetch = jest.fn<typeof fetch>().mockResolvedValue(
-      new Response("I'm a teapot!", {
-        status: 418,
-        statusText: "I'm a teapot!",
-      }),
-    );
-
-    const fetchPromise = getResourceInfo("https://arbitrary.pod/resource", {
-      fetch: mockFetch,
-    });
-
-    await expect(fetchPromise).rejects.toBeInstanceOf(FetchError);
-  });
 });
 
 describe("isContainer", () => {

diff --git a/src/resource/resource.ts b/src/resource/resource.ts
@@ -19,6 +19,14 @@
 // SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
 
+// This linter exception is introduced for legacy reasons.
+/* eslint-disable max-classes-per-file */
+
+import { ClientHttpError } from "@inrupt/solid-client-errors";
+import type {
+  WithProblemDetails,
+  ProblemDetails,
+} from "@inrupt/solid-client-errors";
 import type {
   UrlString,
   Url,
@@ -261,10 +269,31 @@ export function getEffectiveAccess(
  * Extends the regular JavaScript error object with access to the status code and status message.
  * @since 1.2.0
  */
-export class FetchError extends SolidClientError {
+export class FetchError extends SolidClientError implements WithProblemDetails {
   /** @since 1.3.0 */
   public readonly response: Response & { ok: false };
 
+  private httpError: ClientHttpError;
+
+  constructor(
+    message: string,
+    errorResponse: Response & { ok: false },
+    responseBody?: string,
+  ) {
+    super(message);
+    this.response = errorResponse;
+    if (typeof responseBody === "string") {
+      this.httpError = new ClientHttpError(
+        errorResponse,
+        responseBody,
+        message,
+      );
+    } else {
+      // If no response body is provided, defaults are applied.
+      this.httpError = new ClientHttpError(errorResponse, "", message);
+    }
+  }
+
   get statusCode(): number {
     return this.response.status;
   }
@@ -273,8 +302,7 @@ export class FetchError extends SolidClientError {
     return this.response.statusText;
   }
 
-  constructor(message: string, errorResponse: Response & { ok: false }) {
-    super(message);
-    this.response = errorResponse;
+  get problemDetails(): ProblemDetails {
+    return this.httpError.problemDetails;
   }
 }
diff --git a/src/resource/solidDataset.test.ts b/src/resource/solidDataset.test.ts
@@ -278,7 +278,7 @@ describe("responseToSolidDataset", () => {
     const dataPromise = new Promise<string>((resolve) => {
       resolveDataPromise = resolve;
     });
-    jest.spyOn(response, "text").mockReturnValueOnce(dataPromise);
+    jest.spyOn(response, "text").mockReturnValue(dataPromise);
 
     const onErrorHandlers: Array<Parameters<Parser["onError"]>[0]> = [];
     const mockParser: Parser = {