From 083e8a869b05b7712b66dd047b4b98a66957b665 Mon Sep 17 00:00:00 2001
From: Jack Michaud <jack@lomz.me>
Date: Fri, 6 Jan 2023 10:13:54 -0500
Subject: [PATCH 1/3] test: add unittest tests for edge cases in OasstApiClient

Fixes #354
---
 .../contract/oasst_api_contract_tests.cy.ts   | 69 +++++++++++++++++--
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/website/cypress/contract/oasst_api_contract_tests.cy.ts b/website/cypress/contract/oasst_api_contract_tests.cy.ts
index ff5bb156..2abbdf41 100644
--- a/website/cypress/contract/oasst_api_contract_tests.cy.ts
+++ b/website/cypress/contract/oasst_api_contract_tests.cy.ts
@@ -1,4 +1,4 @@
-import OasstApiClient from "src/lib/oasst_api_client";
+import OasstApiClient, { OasstError } from "src/lib/oasst_api_client";
 
 describe("Contract test for Oasst API", function () {
   // Assumes this is running the mock server.
@@ -23,7 +23,68 @@ describe("Contract test for Oasst API", function () {
     expect(await oasstApiClient.ackTask(task.id, "321")).to.be.null;
   });
 
-  // TODO(#354): Add test for 204
-  // TODO(#354): Add test for parsing >=300, throwing an OasstError
-  // TODO(#354): Add test for parsing >=300, throwing a generic error
+  // Note: Below here are unittests for OasstApiClient, not contract tests. They still fit here because
+  // the other contract tests are also testing the OasstApiClient.
+  const callApiMethod = () => {
+    return oasstApiClient.ackTask("", "");
+  };
+
+  it("should return null for a 204 response", async () => {
+    const mockFetch = cy.stub(global, "fetch").resolves({
+      status: 204,
+    });
+
+    const result = await callApiMethod();
+    assert.isNull(result);
+
+    mockFetch.restore();
+  });
+
+  it("should throw an OasstError with data from the response for a non-2XX response with a valid OasstError response", async () => {
+    const mockFetch = cy.stub(global, "fetch").resolves({
+      status: 400,
+      text: () =>
+        // Note: this is vulnerable to interface changes in the Oasst API.
+        // The python tests use a Pydantic model to ensure this object is always valid,
+        // but we don't have that here.
+        // This could be a case for generating Zod schemas from OpenAPI.
+        Promise.resolve(
+          JSON.stringify({
+            message: "error message",
+            error_code: 1000,
+          })
+        ),
+    });
+
+    try {
+      await callApiMethod();
+      assert.fail();
+    } catch (error) {
+      assert.instanceOf(error, OasstError);
+      if (error instanceof OasstError) {
+        assert.equal(error.errorCode, 1000);
+        assert.equal(error.message, "error message");
+        assert.equal(error.httpStatusCode, 400);
+      }
+    }
+
+    mockFetch.restore();
+  });
+
+  it("should throw a generic OasstError with the text from the response for a non-2XX response with an unknown format", async () => {
+    const mockFetch = cy.stub(global, "fetch").resolves({
+      status: 400,
+      text: () => Promise.resolve("error message"),
+    });
+
+    try {
+      await callApiMethod();
+      assert.fail();
+    } catch (error) {
+      assert.instanceOf(error, OasstError);
+      assert.equal(error.message, "error message");
+    }
+
+    mockFetch.restore();
+  });
 });

From a9a3fc4e4ac6123ff31ee9c83e7dd0420a7c5ff6 Mon Sep 17 00:00:00 2001
From: Jack Michaud <jack@lomz.me>
Date: Fri, 6 Jan 2023 10:15:07 -0500
Subject: [PATCH 2/3] fix: handle OasstError correctly in OasstApiClient

---
 website/src/lib/oasst_api_client.ts | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/website/src/lib/oasst_api_client.ts b/website/src/lib/oasst_api_client.ts
index 45a0859e..8b2ade72 100644
--- a/website/src/lib/oasst_api_client.ts
+++ b/website/src/lib/oasst_api_client.ts
@@ -1,6 +1,6 @@
 import { JWT } from "next-auth/jwt";
 
-class OasstError {
+export class OasstError {
   message: string;
   errorCode: number;
   httpStatusCode: number;
@@ -31,12 +31,13 @@ export default class OasstApiClient {
 
     if (resp.status >= 300) {
       const errorText = await resp.text();
+      let error: any;
       try {
-        const error = JSON.parse(errorText);
-        throw new OasstError(error.message, error.error_code, resp.status);
+        error = JSON.parse(errorText);
       } catch (e) {
         throw new OasstError(errorText, 0, resp.status);
       }
+      throw new OasstError(error.message, error.error_code, resp.status);
     }
 
     return await resp.json();

From 239c10c2dc02ac6bd32867cf5f2233813604ae69 Mon Sep 17 00:00:00 2001
From: Jack Michaud <jack@lomz.me>
Date: Fri, 6 Jan 2023 10:21:23 -0500
Subject: [PATCH 3/3] fix: export OasstError

---
 website/cypress/contract/oasst_api_contract_tests.cy.ts | 2 +-
 website/src/lib/oasst_api_client.ts                     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/website/cypress/contract/oasst_api_contract_tests.cy.ts b/website/cypress/contract/oasst_api_contract_tests.cy.ts
index 113e5e9c..47d43c34 100644
--- a/website/cypress/contract/oasst_api_contract_tests.cy.ts
+++ b/website/cypress/contract/oasst_api_contract_tests.cy.ts
@@ -1,4 +1,4 @@
-import { OasstApiClient } from "src/lib/oasst_api_client";
+import { OasstApiClient, OasstError } from "src/lib/oasst_api_client";
 
 describe("Contract test for Oasst API", function () {
   // Assumes this is running the mock server.
diff --git a/website/src/lib/oasst_api_client.ts b/website/src/lib/oasst_api_client.ts
index 3755d676..4cf891e1 100644
--- a/website/src/lib/oasst_api_client.ts
+++ b/website/src/lib/oasst_api_client.ts
@@ -5,7 +5,7 @@ declare global {
   var oasstApiClient: OasstApiClient | undefined;
 }
 
-class OasstError {
+export class OasstError {
   message: string;
   errorCode: number;
   httpStatusCode: number;