Merge pull request #447 from LAION-AI/jm/tests-for-javascript-oasst-api-client

tests + fix: #354 Add tests for OasstApiClient and fix handling of OasstError

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.
@@ -44,7 +44,68 @@ describe("Contract test for Oasst API", function () {
     ).to.be.not.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();
+  });
 });

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;
@@ -36,12 +36,13 @@ export 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();