From ae5d5f077bcc6305bebe5cb5cde31cf588ef0c20 Mon Sep 17 00:00:00 2001
From: Matt Fedderly <mfedderly@palantir.com>
Date: Thu, 29 Feb 2024 14:58:58 -0500
Subject: [PATCH 1/2] Handle error status codes when making network calls

---
 .../src/client/createOpenApiRequest.test.ts   | 39 +++++++++++++++++++
 .../src/client/createOpenApiRequest.ts        | 30 ++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/packages/shared.net/src/client/createOpenApiRequest.test.ts b/packages/shared.net/src/client/createOpenApiRequest.test.ts
index 58735b554..ba0593bd0 100644
--- a/packages/shared.net/src/client/createOpenApiRequest.test.ts
+++ b/packages/shared.net/src/client/createOpenApiRequest.test.ts
@@ -16,6 +16,7 @@
 
 import type { OpenApiRequest } from "@osdk/gateway/types";
 import { describe, expect, expectTypeOf, it, vi } from "vitest";
+import { PalantirApiError } from "../PalantirApiError.js";
 import { createOpenApiRequest } from "./createOpenApiRequest.js";
 
 describe("createOpenApiRequest", () => {
@@ -23,6 +24,7 @@ describe("createOpenApiRequest", () => {
     const mockFetch = vi.fn();
 
     mockFetch.mockResolvedValue({
+      ok: true,
       json: () => Promise.resolve({ test: 1 }),
     });
 
@@ -66,6 +68,7 @@ describe("createOpenApiRequest", () => {
     const mockFetch = vi.fn();
 
     mockFetch.mockResolvedValue({
+      ok: true,
       json: () => Promise.resolve({ test: 1 }),
     });
 
@@ -97,6 +100,7 @@ describe("createOpenApiRequest", () => {
     const mockFetch = vi.fn();
 
     mockFetch.mockResolvedValue({
+      ok: true,
       json: () => Promise.resolve({ test: 1 }),
     });
 
@@ -123,6 +127,7 @@ describe("createOpenApiRequest", () => {
     const mockFetch = vi.fn();
 
     mockFetch.mockResolvedValue({
+      ok: true,
       json: () => Promise.resolve({ test: 1 }),
     });
 
@@ -150,6 +155,7 @@ describe("createOpenApiRequest", () => {
 
     const stream = new ReadableStream();
     mockFetch.mockResolvedValue({
+      ok: true,
       body: stream,
     });
 
@@ -194,6 +200,7 @@ describe("createOpenApiRequest", () => {
 
     const blob = new Blob();
     mockFetch.mockResolvedValue({
+      ok: true,
       blob: () => Promise.resolve(blob),
     });
 
@@ -234,4 +241,36 @@ describe("createOpenApiRequest", () => {
       },
     );
   });
+
+  it("handles error status codes", async () => {
+    const mockFetch = vi.fn();
+
+    const mockResponse: ReturnType<typeof fetch> = Promise.resolve({
+      ok: false,
+      status: 500,
+      json: () =>
+        Promise.resolve({
+          errorCode: "INTERNAL",
+          errorName: "Default:Internal",
+          errorInstanceId: "00000000-0000-0000-0000-000000000000",
+          parameters: {},
+        }),
+    } as Response);
+
+    mockFetch.mockImplementationOnce(() => mockResponse);
+
+    const request = createOpenApiRequest<{}>(
+      "http://example.com",
+      mockFetch,
+      undefined,
+      false,
+    );
+
+    try {
+      await request("POST", "/");
+      expect.fail();
+    } catch (error) {
+      expect(error).toBeInstanceOf(PalantirApiError);
+    }
+  });
 });
diff --git a/packages/shared.net/src/client/createOpenApiRequest.ts b/packages/shared.net/src/client/createOpenApiRequest.ts
index 676b2e127..55bcc6229 100644
--- a/packages/shared.net/src/client/createOpenApiRequest.ts
+++ b/packages/shared.net/src/client/createOpenApiRequest.ts
@@ -15,6 +15,8 @@
  */
 
 import type { OpenApiRequest } from "@osdk/gateway/types";
+import { PalantirApiError } from "../PalantirApiError.js";
+import { UnknownError } from "../UnknownError.js";
 import { replaceHttpIfNotLocalhost } from "../util/index.js";
 
 export function createOpenApiRequest<
@@ -73,6 +75,12 @@ export function createOpenApiRequest<
       headers: headersInit,
     });
 
+    // error status codes are not thrown by fetch automatically,
+    // we have to look at the ok property and behave accordingly
+    if (!response.ok) {
+      throw await convertError(response);
+    }
+
     if (responseMediaType && responseMediaType === "*/*") {
       if (asReadableStream) {
         return response.body;
@@ -104,3 +112,25 @@ function withHttps(url: string): string {
     ? replaceHttpIfNotLocalhost(url)
     : `${httpsProtocol}${url}`;
 }
+
+async function convertError(response: Response) {
+  try {
+    const convertedError = await response.json();
+    return new PalantirApiError(
+      convertedError.message,
+      convertedError.errorName,
+      convertedError.errorCode,
+      response.status,
+      convertedError.errorInstanceId,
+      convertedError.parameters,
+    );
+  } catch (e) {
+    if (e instanceof Error) {
+      return new UnknownError(e.message, "UNKNOWN");
+    }
+    return new UnknownError(
+      "Unable to parse error response",
+      "UNKNOWN",
+    );
+  }
+}

From 4b064a5d55c47060db48bb29a65c2d332a80aac9 Mon Sep 17 00:00:00 2001
From: svc-changelog <svc-changelog@palantir.com>
Date: Thu, 29 Feb 2024 20:34:44 +0000
Subject: [PATCH 2/2] Add generated changelog entries

---
 packages/shared.net/changelog/@unreleased/pr-103.v2.yml | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 packages/shared.net/changelog/@unreleased/pr-103.v2.yml

diff --git a/packages/shared.net/changelog/@unreleased/pr-103.v2.yml b/packages/shared.net/changelog/@unreleased/pr-103.v2.yml
new file mode 100644
index 000000000..cc73b3bbe
--- /dev/null
+++ b/packages/shared.net/changelog/@unreleased/pr-103.v2.yml
@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: Handle error status codes when making network calls
+  links:
+  - https://github.com/palantir/osdk-ts/pull/103