Merge pull request #155 from PermanentOrg/89-update-return-types

Throw on non-ok server responses

CHANGELOG.md

@@ -6,7 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-- This package no longer supports Node v14
+## Changed
+- This package no longer supports Node v14.
+- The `deleteFolder` and `deleteArchiveRecord` SDK methods now return void (was boolean).
+- All SDK methods now throw if the server responds with a non-OK status.
+
+## Added
+- The `HttpResponseError` type now exists.
 
 ## [0.6.0] - 2023-05-18
 ## Changed

src/api/__tests__/deleteFolderVo.test.ts

@@ -1,40 +1,37 @@
 import nock from 'nock';
 import { deleteFolderVo } from '..';
+import { HttpResponseError } from '../../errors';
 
 describe('deleteFolderVo', () => {
-  it('should return true when a folder is deleted', async () => {
+  it('should not throw when a folder is deleted', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/folder/delete?folderId=1',
       )
       .reply(200);
 
-    const result = await deleteFolderVo(
+    await expect(deleteFolderVo(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(true);
+    )).resolves.not.toThrow();
   });
 
-  it('should return false when a folder is not deleted', async () => {
+  it('should throw an error when when a 500 status is returned', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/folder/delete?folderId=1',
       )
       .reply(500);
 
-    const result = await deleteFolderVo(
+    await expect(deleteFolderVo(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(false);
+    )).rejects.toThrow(HttpResponseError);
   });
 });

src/api/__tests__/deleteRecordVo.test.ts

@@ -1,40 +1,37 @@
 import nock from 'nock';
 import { deleteRecordVo } from '..';
+import { HttpResponseError } from '../../errors';
 
 describe('deleteArchiveRecord', () => {
-  it('should return true when a record is deleted', async () => {
+  it('should not throw when a record is deleted', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/record/delete?recordId=1',
       )
       .reply(200);
 
-    const result = await deleteRecordVo(
+    await expect(deleteRecordVo(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(true);
+    )).resolves.not.toThrow();
   });
 
-  it('should return false when a record is not deleted', async () => {
+  it('should throw an error when when a 500 status is returned', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/record/delete?recordId=1',
       )
       .reply(500);
 
-    const result = await deleteRecordVo(
+    await expect(deleteRecordVo(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(false);
+    )).rejects.toThrow(HttpResponseError);
   });
 });

src/api/deleteFolderVo.ts

@@ -5,17 +5,13 @@ import type { ClientConfiguration } from '../types';
 export const deleteFolderVo = async (
   clientConfiguration: ClientConfiguration,
   folderId: number,
-): Promise<boolean> => {
+): Promise<void> => {
   const queryParams = new URLSearchParams({
     folderId: `${folderId}`,
   });
-  const response = await makePermanentApiCall(
+  await makePermanentApiCall(
     clientConfiguration,
     `/folder/delete?${queryParams.toString()}`,
     { method: 'DELETE' },
   );
-  if (response.status === 200) {
-    return true;
-  }
-  return false;
 };

src/api/deleteRecordVo.ts

@@ -5,17 +5,13 @@ import type { ClientConfiguration } from '../types';
 export const deleteRecordVo = async (
   clientConfiguration: ClientConfiguration,
   recordId: number,
-): Promise<boolean> => {
+): Promise<void> => {
   const queryParams = new URLSearchParams({
     recordId: `${recordId}`,
   });
-  const response = await makePermanentApiCall(
+  await makePermanentApiCall(
     clientConfiguration,
     `/record/delete?${queryParams.toString()}`,
     { method: 'DELETE' },
   );
-  if (response.status === 200) {
-    return true;
-  }
-  return false;
 };

src/errors/HttpResponseError.ts

@@ -0,0 +1,8 @@
+export class HttpResponseError extends Error {
+  public readonly statusCode: number;
+
+  public constructor(statusCode: number, message: string) {
+    super(message);
+    this.statusCode = statusCode;
+  }
+}

src/errors/index.ts

@@ -1 +1,2 @@
+export * from './HttpResponseError';
 export * from './ValidationError';

src/sdk/__tests__/deleteArchiveRecord.test.ts

@@ -1,40 +1,37 @@
 import nock from 'nock';
 import { deleteArchiveRecord } from '..';
+import { HttpResponseError } from '../../errors';
 
 describe('deleteArchiveRecord', () => {
-  it('should return true when a record is deleted', async () => {
+  it('should not throw on successful deletion', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/record/delete?recordId=1',
       )
       .reply(200);
 
-    const result = await deleteArchiveRecord(
+    await expect(deleteArchiveRecord(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(true);
+    )).resolves.not.toThrow();
   });
 
-  it('should return false when a record is not deleted', async () => {
+  it('should throw an error when receiving a 500 response', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/record/delete?recordId=1',
       )
       .reply(500);
 
-    const result = await deleteArchiveRecord(
+    await expect(deleteArchiveRecord(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(false);
+    )).rejects.toThrow(HttpResponseError);
   });
 });

src/sdk/__tests__/deleteFolder.test.ts

@@ -1,40 +1,37 @@
 import nock from 'nock';
 import { deleteFolder } from '..';
+import { HttpResponseError } from '../../errors/HttpResponseError';
 
 describe('deleteFolder', () => {
-  it('should return true when a folder is deleted', async () => {
+  it('should not throw when a folder is deleted', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/folder/delete?folderId=1',
       )
       .reply(200);
 
-    const result = await deleteFolder(
+    await expect(deleteFolder(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(true);
+    )).resolves.not.toThrow();
   });
 
-  it('should return false when a folder is not deleted', async () => {
+  it('should throw an error when receiving a 500 response', async () => {
     nock('https://permanent.local')
       .delete(
         '/api/folder/delete?folderId=1',
       )
       .reply(500);
 
-    const result = await deleteFolder(
+    await expect(deleteFolder(
       {
         bearerToken: '12345',
         baseUrl: 'https://permanent.local/api',
       },
       1,
-    );
-
-    expect(result).toBe(false);
+    )).rejects.toThrow(HttpResponseError);
   });
 });

src/sdk/deleteArchiveRecord.ts

@@ -6,7 +6,7 @@ import type {
 export const deleteArchiveRecord = async (
   clientConfiguration: ClientConfiguration,
   archiveRecordId: number,
-): Promise<boolean> => deleteRecordVo(
+): Promise<void> => deleteRecordVo(
   clientConfiguration,
   archiveRecordId,
 );

src/sdk/deleteFolder.ts

@@ -6,7 +6,7 @@ import type {
 export const deleteFolder = async (
   clientConfiguration: ClientConfiguration,
   folderId: number,
-): Promise<boolean> => deleteFolderVo(
+): Promise<void> => deleteFolderVo(
   clientConfiguration,
   folderId,
 );

src/utils/makePermanentApiCall.ts

@@ -1,4 +1,5 @@
 import fetch from 'node-fetch';
+import { HttpResponseError } from '../errors';
 import type {
   RequestInit,
   Response,
@@ -38,5 +39,11 @@ export const makePermanentApiCall = async (
       headers,
     },
   );
+  if (!response.ok) {
+    throw new HttpResponseError(
+      response.status,
+      await response.text(),
+    );
+  }
   return response;
 };