Refactor http client to introduce custom error types (#113)

* start refactor * add deps * Add msw and more tests * Add todo comment * fix deps * Add validation errors * Add sinon pkg * Refactor client sendMessage getOfferings getExchange getExchanges getPfiServiceEndpoint * Add mocks and tests for sendMessage, getOfferings, and getPfiServiceEndpoint * Add tests for getExchange and getExchanges * small revert * small import fix * small revert * remove httpresponse return val from sendmessage * Rename InvalidServiceEndpointError to MissingServiceEndpointError * Remove DataResponse type * Remove msw in favour of sinon stub * Remove msw pkg * Add changeset * Update changeset --------- Co-authored-by: Moe Jangda <[email protected]>
TBD54566975 · Dec 20, 2023 · 9e1015e · 9e1015e
1 parent c3610ed
commit 9e1015e
Show file tree

Hide file tree

Showing 12 changed files with 852 additions and 497 deletions.
diff --git a/.changeset/four-mangos-swim.md b/.changeset/four-mangos-swim.md
@@ -0,0 +1,5 @@
+---
+"@tbdex/http-client": minor
+---
+
+Introduces custom errors types and breaking changes: functions now throw instead of return on failure
diff --git a/packages/http-client/package.json b/packages/http-client/package.json
@@ -50,16 +50,17 @@
   },
   "dependencies": {
     "@tbdex/protocol": "workspace:*",
+    "@web5/common": "0.2.1",
     "@web5/crypto": "0.2.2",
     "@web5/dids": "0.2.2",
-    "@web5/common": "0.2.1",
     "query-string": "8.1.0"
   },
   "devDependencies": {
     "@playwright/test": "1.34.3",
     "@types/chai": "4.3.5",
     "@types/eslint": "8.37.0",
     "@types/mocha": "10.0.1",
+    "@types/sinon": "^17.0.2",
     "@typescript-eslint/eslint-plugin": "5.59.0",
     "@typescript-eslint/parser": "5.59.0",
     "chai": "4.3.10",

diff --git a/packages/http-client/src/client.ts b/packages/http-client/src/client.ts
@@ -1,4 +1,4 @@
-import type { DataResponse, ErrorDetail, ErrorResponse, HttpResponse } from './types.js'
+import type { ErrorDetail } from './types.js'
 import type { PortableDid } from '@web5/dids'
 import type {
   ResourceMetadata,
@@ -12,7 +12,7 @@ import type {
 import { resolveDid, Offering, Resource, Message, Crypto } from '@tbdex/protocol'
 import { utils as didUtils } from '@web5/dids'
 import { Convert } from '@web5/common'
-
+import { RequestError, ResponseError, InvalidDidError, MissingServiceEndpointError } from './errors/index.js'
 import queryString from 'query-string'
 
 /**
@@ -27,7 +27,7 @@ export class TbdexHttpClient {
    * @throws if recipient DID resolution fails
    * @throws if recipient DID does not have a PFI service entry
    */
-  static async sendMessage<T extends MessageKind>(opts: SendMessageOptions<T>): Promise<HttpResponse | ErrorResponse> {
+  static async sendMessage<T extends MessageKind>(opts: SendMessageOptions<T>): Promise<void> {
     const { message } = opts
     const jsonMessage: MessageModel<T> = message instanceof Message ? message.toJSON() : message
 
@@ -45,20 +45,12 @@ export class TbdexHttpClient {
         body    : JSON.stringify(jsonMessage)
       })
     } catch(e) {
-      throw new Error(`Failed to send message to ${pfiDid}. Error: ${e.message}`)
+      throw new RequestError({ message: `Failed to send message to ${pfiDid}`, recipientDid: pfiDid, url: apiRoute, cause: e })
     }
 
-    const { status, headers } = response
-    if (status === 202) {
-      return { status, headers }
-    } else {
-      // TODO: figure out what happens if this fails. do we need to try/catch?
-      const responseBody: { errors: ErrorDetail[] } = await response.json()
-      return {
-        status  : response.status,
-        headers : response.headers,
-        errors  : responseBody.errors
-      }
+    if (!response.ok) {
+      const errorDetails = await response.json() as ErrorDetail[]
+      throw new ResponseError({ statusCode: response.status, details: errorDetails, recipientDid: pfiDid, url: apiRoute })
     }
   }
 
@@ -114,9 +106,10 @@ export class TbdexHttpClient {
 
   /**
    * gets offerings from the pfi provided
-   * @param _opts - options
+   * @param opts - options
+   * @beta
    */
-  static async getOfferings(opts: GetOfferingsOptions): Promise<DataResponse<Offering[]> | ErrorResponse> {
+  static async getOfferings(opts: GetOfferingsOptions): Promise<Offering[]> {
     const { pfiDid , filter } = opts
 
     const pfiServiceEndpoint = await TbdexHttpClient.getPfiServiceEndpoint(pfiDid)
@@ -127,37 +120,30 @@ export class TbdexHttpClient {
     try {
       response = await fetch(apiRoute)
     } catch(e) {
-      throw new Error(`Failed to get offerings from ${pfiDid}. Error: ${e.message}`)
+      throw new RequestError({ message: `Failed to get offerings from ${pfiDid}`, recipientDid: pfiDid, url: apiRoute, cause: e })
     }
 
     const data: Offering[] = []
 
-    if (response.status === 200) {
-      const responseBody = await response.json() as { data: ResourceModel<'offering'>[] }
-      for (let jsonResource of responseBody.data) {
-        const resource = await Resource.parse(jsonResource)
-        data.push(resource)
-      }
+    if (!response.ok) {
+      const errorDetails = await response.json() as ErrorDetail[]
+      throw new ResponseError({ statusCode: response.status, details: errorDetails, recipientDid: pfiDid, url: apiRoute })
+    }
 
-      return {
-        status  : response.status,
-        headers : response.headers,
-        data    : data
-      }
-    } else {
-      return {
-        status  : response.status,
-        headers : response.headers,
-        errors  : await response.json() as ErrorDetail[]
-      } as ErrorResponse
+    const responseBody = await response.json() as { data: ResourceModel<'offering'>[] }
+    for (let jsonResource of responseBody.data) {
+      const resource = await Resource.parse(jsonResource)
+      data.push(resource)
     }
+
+    return data
   }
 
   /**
    * get a specific exchange from the pfi provided
    * @param _opts - options
    */
-  static async getExchange(opts: GetExchangeOptions): Promise<DataResponse<MessageKindClass[]> | ErrorResponse> {
+  static async getExchange(opts: GetExchangeOptions): Promise<MessageKindClass[]> {
     const { pfiDid, exchangeId, did } = opts
 
     const pfiServiceEndpoint = await TbdexHttpClient.getPfiServiceEndpoint(pfiDid)
@@ -172,40 +158,34 @@ export class TbdexHttpClient {
         }
       })
     } catch(e) {
-      throw new Error(`Failed to get offerings from ${pfiDid}. Error: ${e.message}`)
+      throw new RequestError({ message: `Failed to get exchange from ${pfiDid}`, recipientDid: pfiDid, url: apiRoute, cause: e })
     }
 
     const data: MessageKindClass[] = []
 
-    if (response.status === 200) {
-      const responseBody = await response.json() as { data: MessageModel<MessageKind>[] }
-      for (let jsonMessage of responseBody.data) {
-        const message = await Message.parse(jsonMessage)
-        data.push(message)
-      }
+    if (!response.ok) {
+      const errorDetails = await response.json() as ErrorDetail[]
+      throw new ResponseError({ statusCode: response.status, details: errorDetails, recipientDid: pfiDid, url: apiRoute })
+    }
 
-      return {
-        status  : response.status,
-        headers : response.headers,
-        data    : data
-      }
-    } else {
-      return {
-        status  : response.status,
-        headers : response.headers,
-        errors  : await response.json() as ErrorDetail[]
-      } as ErrorResponse
+    const responseBody = await response.json() as { data: MessageModel<MessageKind>[] }
+    for (let jsonMessage of responseBody.data) {
+      const message = await Message.parse(jsonMessage)
+      data.push(message)
     }
+
+    return data
+
   }
 
   /**
    * returns all exchanges created by requester
    * @param _opts - options
    */
-  static async getExchanges(opts: GetExchangesOptions): Promise<DataResponse<MessageKindClass[][]> | ErrorResponse> {
+  static async getExchanges(opts: GetExchangesOptions): Promise<MessageKindClass[][]> {
     const { pfiDid, filter, did } = opts
-    const pfiServiceEndpoint = await TbdexHttpClient.getPfiServiceEndpoint(pfiDid)
 
+    const pfiServiceEndpoint = await TbdexHttpClient.getPfiServiceEndpoint(pfiDid)
     const queryParams = filter ? `?${queryString.stringify(filter)}`: ''
     const apiRoute = `${pfiServiceEndpoint}/exchanges${queryParams}`
     const requestToken = await TbdexHttpClient.generateRequestToken(did)
@@ -218,50 +198,50 @@ export class TbdexHttpClient {
         }
       })
     } catch(e) {
-      throw new Error(`Failed to get exchanges from ${pfiDid}. Error: ${e.message}`)
+      throw new RequestError({ message: `Failed to get exchanges from ${pfiDid}`, recipientDid: pfiDid, url: apiRoute, cause: e })
     }
 
     const exchanges: MessageKindClass[][] = []
 
-    if (response.status === 200) {
-      const responseBody = await response.json() as { data: MessageModel<MessageKind>[][] }
-      for (let jsonExchange of responseBody.data) {
-        const exchange: MessageKindClass[] = []
+    if (!response.ok) {
+      const errorDetails = await response.json() as ErrorDetail[]
+      throw new ResponseError({ statusCode: response.status, details: errorDetails, recipientDid: pfiDid, url: apiRoute })
+    }
 
-        for (let jsonMessage of jsonExchange) {
-          const message = await Message.parse(jsonMessage)
-          exchange.push(message)
-        }
+    const responseBody = await response.json() as { data: MessageModel<MessageKind>[][] }
+    for (let jsonExchange of responseBody.data) {
+      const exchange: MessageKindClass[] = []
 
-        exchanges.push(exchange)
+      for (let jsonMessage of jsonExchange) {
+        const message = await Message.parse(jsonMessage)
+        exchange.push(message)
       }
 
-      return {
-        status  : response.status,
-        headers : response.headers,
-        data    : exchanges
-      }
-    } else {
-      return {
-        status  : response.status,
-        headers : response.headers,
-        errors  : await response.json() as ErrorDetail[]
-      } as ErrorResponse
+      exchanges.push(exchange)
     }
+
+    return exchanges
   }
 
   /**
    * returns the PFI service entry from the DID Doc of the DID provided
    * @param did - the pfi's DID
    */
   static async getPfiServiceEndpoint(did: string) {
-    const didDocument = await resolveDid(did)
-    const [ didService ] = didUtils.getServices({ didDocument, type: 'PFI' })
+    try {
+      const didDocument = await resolveDid(did)
+      const [ didService ] = didUtils.getServices({ didDocument, type: 'PFI' })
+
+      if (!didService?.serviceEndpoint) {
+        throw new MissingServiceEndpointError(`${did} has no PFI service entry`)
+      }
 
-    if (didService?.serviceEndpoint) {
       return didService.serviceEndpoint
-    } else {
-      throw new Error(`${did} has no PFI service entry`)
+    } catch (e) {
+      if (e instanceof MissingServiceEndpointError) {
+        throw e
+      }
+      throw new InvalidDidError(e)
     }
   }
 

diff --git a/packages/http-client/src/errors/index.ts b/packages/http-client/src/errors/index.ts
@@ -0,0 +1,3 @@
+export { RequestError } from './request-error.js'
+export { ResponseError } from './response-error.js'
+export { ValidationError, InvalidDidError, MissingServiceEndpointError } from './validation-error.js'
diff --git a/packages/http-client/src/errors/request-error.ts b/packages/http-client/src/errors/request-error.ts
@@ -0,0 +1,25 @@
+export type RequestErrorParams = {
+  message: string
+  recipientDid: string
+  url?: string
+  cause?: unknown
+}
+
+/**
+ * Error thrown when making HTTP requests
+ * @beta
+ */
+export class RequestError extends Error {
+  public readonly recipientDid: string
+  public readonly url: string
+
+  constructor(params: RequestErrorParams) {
+    super(params.message, { cause: params.cause })
+
+    this.name = this.constructor.name
+    this.recipientDid = params.recipientDid
+    this.url = params.url
+
+    Object.setPrototypeOf(this, RequestError.prototype)
+  }
+}
diff --git a/packages/http-client/src/errors/response-error.ts b/packages/http-client/src/errors/response-error.ts
@@ -0,0 +1,31 @@
+import type { ErrorDetail } from '../types.js'
+
+export type ResponseErrorParams = {
+  statusCode: number
+  details: ErrorDetail[]
+  recipientDid: string
+  url: string
+}
+
+/**
+ * Error thrown when getting HTTP responses
+ * @beta
+ */
+export class ResponseError extends Error {
+  public readonly statusCode: number
+  public readonly details: ErrorDetail[]
+  public readonly recipientDid: string
+  public readonly url: string
+
+  constructor(params: ResponseErrorParams) {
+    super()
+
+    this.name = this.constructor.name
+    this.statusCode = params.statusCode
+    this.details = params.details
+    this.recipientDid = params.recipientDid
+    this.url = params.url
+
+    Object.setPrototypeOf(this, ResponseError.prototype)
+  }
+}
diff --git a/packages/http-client/src/errors/validation-error.ts b/packages/http-client/src/errors/validation-error.ts
@@ -0,0 +1,41 @@
+export type ValidationErrorParams = {
+  message: string
+}
+
+/**
+ * Error thrown when validating data
+ * @beta
+ */
+export class ValidationError extends Error {
+  constructor(message: string) {
+    super(message)
+
+    this.name = this.constructor.name
+
+    Object.setPrototypeOf(this, ValidationError.prototype)
+  }
+}
+
+/**
+ * Error thrown when a DID is invalid
+ * @beta
+ */
+export class InvalidDidError extends ValidationError {
+  constructor(message: string) {
+    super(message)
+
+    Object.setPrototypeOf(this, InvalidDidError.prototype)
+  }
+}
+
+/**
+ * Error thrown when a PFI's service endpoint can't be found
+ * @beta
+ */
+export class MissingServiceEndpointError extends ValidationError {
+  constructor(message: string) {
+    super(message)
+
+    Object.setPrototypeOf(this, MissingServiceEndpointError.prototype)
+  }
+}
diff --git a/packages/http-client/src/types.ts b/packages/http-client/src/types.ts
@@ -35,15 +35,6 @@ export type HttpResponse = {
   headers: Headers
 }
 
-/**
- * HTTP Response with data
- * @beta
- */
-export type DataResponse<T> = HttpResponse & {
-  data: T
-  errors?: never
-}
-
 /**
  * HTTP Response with errors
  * @beta