Improve logs and errors in HttpClient (#688)

* Add fetchWithRetries

* wip

* Match fetch interface

* Add tests for fetchRetry

* Pass custom delay in HTTPClient tests

* Add additional tests for HttpClient

* Fix conflict

* Change maxCalls to 5 by default

* Change info to trace in HttpClient

* Add custom HttpError class

* Add url to logs

* Improve HttpError

* Pass url to HttpError

packages/common-nodejs/src/http/HttpClient.test.ts

@@ -1,7 +1,7 @@
 import { MockObject, expect, mockFn, mockObject } from 'earl'
 import { ZodError, z } from 'zod'
 import { Logger } from '../logger/index.js'
-import { HttpClient } from './HttpClient.js'
+import { HttpClient, HttpError } from './HttpClient.js'
 import { HttpServerMock, PostBody, getResponseSchema, postBodySchema } from './HttpServer.mock.js'
 
 describe(HttpClient.name, () => {
@@ -28,7 +28,7 @@ describe(HttpClient.name, () => {
       const httpClient = new HttpClient(logger, { delay: 0 })
 
       await httpClient.get(url, getResponseSchema)
-      expect(logger.info).toHaveBeenOnlyCalledWith('[HttpClient] GET request', { url })
+      expect(logger.trace).toHaveBeenOnlyCalledWith(`[HttpClient] GET request - ${url}`, { url })
     })
 
     it('throws with invalid schema', async () => {
@@ -44,16 +44,20 @@ describe(HttpClient.name, () => {
 
     it("doesn't retry in case of client error", async () => {
       const httpClient = new HttpClient(Logger.SILENT, { delay: 0 })
-      await expect(() => httpClient.get(httpServer.getUrl('/status?status=400'), getResponseSchema)).toBeRejectedWith(
-        'Failed GET: 400 - {"status":400}',
+      const url = httpServer.getUrl('/status?status=400')
+
+      await expect(() => httpClient.get(url, getResponseSchema)).toBeRejectedWith(
+        `Failed GET ${url}: 400 - {"status":400}`,
       )
       expect(httpServer.requestsCount['/status']).toEqual(1)
     })
 
     it('retries in case of server error', async () => {
       const httpClient = new HttpClient(Logger.SILENT, { delay: 0 })
-      await expect(() => httpClient.get(httpServer.getUrl('/status?status=500'), getResponseSchema)).toBeRejectedWith(
-        'Failed GET: 500 - {"status":500}',
+      const url = httpServer.getUrl('/status?status=500')
+
+      await expect(() => httpClient.get(url, getResponseSchema)).toBeRejectedWith(
+        `Failed GET ${url}: 500 - {"status":500}`,
       )
       expect(httpServer.requestsCount['/status']).toEqual(5)
     })
@@ -78,7 +82,7 @@ describe(HttpClient.name, () => {
       }
 
       expect(await httpClient.post(url, body, postBodySchema)).toEqual(body)
-      expect(logger.info).toHaveBeenOnlyCalledWith('[HttpClient] POST request', { url, body })
+      expect(logger.trace).toHaveBeenOnlyCalledWith(`[HttpClient] POST request - ${url}`, { url, body })
     })
 
     it('throws with invalid schema', async () => {
@@ -95,24 +99,28 @@ describe(HttpClient.name, () => {
 
     it("doesn't retries in case of client error by default", async () => {
       const httpClient = new HttpClient(Logger.SILENT, { delay: 0 })
+      const url = httpServer.getUrl('/post')
       const body: PostBody = {
         status: 400,
       }
 
-      await expect(() => httpClient.post(httpServer.getUrl('/post'), body, postBodySchema)).toBeRejectedWith(
-        'Failed POST: 400 - {"status":400}',
+      await expect(() => httpClient.post(url, body, postBodySchema)).toBeRejectedWith(
+        HttpError,
+        `Failed POST ${url}: 400 - {"status":400}`,
       )
       expect(httpServer.requestsCount['/post']).toEqual(1)
     })
 
     it('retries in case of server error by default', async () => {
       const httpClient = new HttpClient(Logger.SILENT, { delay: 0 })
+      const url = httpServer.getUrl('/post')
       const body: PostBody = {
         status: 500,
       }
 
-      await expect(() => httpClient.post(httpServer.getUrl('/post'), body, postBodySchema)).toBeRejectedWith(
-        'Failed POST: 500 - {"status":500}',
+      await expect(() => httpClient.post(url, body, postBodySchema)).toBeRejectedWith(
+        HttpError,
+        `Failed POST ${url}: 500 - {"status":500}`,
       )
       expect(httpServer.requestsCount['/post']).toEqual(5)
     })
@@ -121,7 +129,7 @@ describe(HttpClient.name, () => {
 
 function getMockLogger(): MockObject<Logger> {
   const mockLogger = mockObject<Logger>({
-    info: mockFn(() => {}),
+    trace: mockFn(() => {}),
     for: (_): Logger => mockLogger,
   })
   return mockLogger

packages/common-nodejs/src/http/HttpClient.ts

@@ -13,7 +13,7 @@ export class HttpClient {
   }
 
   async post<T extends z.ZodTypeAny>(url: string, body: object, schema: T): Promise<z.infer<T>> {
-    this.logger.info('[HttpClient] POST request', { url, body })
+    this.logger.trace(`[HttpClient] POST request - ${url}`, { url, body })
 
     const result = await this.fetchWithRetries(url, {
       method: 'POST',
@@ -23,18 +23,30 @@ export class HttpClient {
       body: JSON.stringify(body),
     })
     if (!result.ok) {
-      throw new Error(`Failed POST: ${result.status} - ${await result.text()}`)
+      throw new HttpError('POST', url, result.status, await result.text())
     }
 
     return schema.parse(await result.json())
   }
 
   async get<T extends z.ZodTypeAny>(url: string, schema: T): Promise<z.infer<T>> {
-    this.logger.info('[HttpClient] GET request', { url })
+    this.logger.trace(`[HttpClient] GET request - ${url}`, { url })
     const result = await this.fetchWithRetries(url)
     if (!result.ok) {
-      throw new Error(`Failed GET: ${result.status} - ${await result.text()}`)
+      throw new HttpError('GET', url, result.status, await result.text())
     }
     return schema.parse(await result.json())
   }
 }
+
+export class HttpError extends Error {
+  constructor(
+    public readonly method: 'POST' | 'GET',
+    public readonly url: string,
+    public readonly status: number,
+    public readonly textResult: string,
+  ) {
+    super(`Failed ${method} ${url}: ${status} - ${textResult}`)
+    this.name = 'HttpError'
+  }
+}