refactor: use Fetch API for HttpService (#2349)

packages/api/jest.config.js

@@ -6,4 +6,5 @@ module.exports = {
   transform: {
     '^.+\\.tsx?$': ['ts-jest', { tsconfig: 'tsconfig.json' }],
   },
-};
+  coverageThreshold: {},
+}

packages/api/src/Domain/Http/FetchRequestHandler.spec.ts

@@ -0,0 +1,144 @@
+import { Environment } from '@standardnotes/models'
+import { HttpVerb } from '@standardnotes/responses'
+import { FetchRequestHandler } from './FetchRequestHandler'
+import { HttpErrorResponseBody, HttpRequest } from '@standardnotes/responses'
+
+import { ErrorMessage } from '../Error'
+
+describe('FetchRequestHandler', () => {
+  const snjsVersion = 'snjsVersion'
+  const appVersion = 'appVersion'
+  const environment = Environment.Web
+  const requestHandler = new FetchRequestHandler(snjsVersion, appVersion, environment)
+
+  it('should create a request', () => {
+    const httpRequest: HttpRequest = {
+      url: 'http://localhost:3000/test',
+      verb: HttpVerb.Get,
+      external: false,
+      authentication: 'authentication',
+      customHeaders: [],
+      params: {
+        key: 'value',
+      },
+    }
+
+    const request = requestHandler['createRequest'](httpRequest)
+
+    expect(request).toBeInstanceOf(Request)
+    expect(request.url).toBe(httpRequest.url)
+    expect(request.method).toBe(httpRequest.verb)
+    expect(request.headers.get('X-SNJS-Version')).toBe(snjsVersion)
+    expect(request.headers.get('X-Application-Version')).toBe(`${Environment[environment]}-${appVersion}`)
+    expect(request.headers.get('Content-Type')).toBe('application/json')
+  })
+
+  it('should get url for url and params', () => {
+    const urlWithoutExistingParams = requestHandler['urlForUrlAndParams']('url', { key: 'value' })
+    expect(urlWithoutExistingParams).toBe('url?key=value')
+
+    const urlWithExistingParams = requestHandler['urlForUrlAndParams']('url?key=value', { key2: 'value2' })
+    expect(urlWithExistingParams).toBe('url?key=value&key2=value2')
+  })
+
+  it('should create request body if not GET', () => {
+    const body = requestHandler['createRequestBody']({
+      url: 'url',
+      verb: HttpVerb.Post,
+      external: false,
+      authentication: 'authentication',
+      customHeaders: [],
+      params: {
+        key: 'value',
+      },
+    })
+
+    expect(body).toBe('{"key":"value"}')
+  })
+
+  it('should not create request body if GET', () => {
+    const body = requestHandler['createRequestBody']({
+      url: 'url',
+      verb: HttpVerb.Get,
+      external: false,
+      authentication: 'authentication',
+      customHeaders: [],
+      params: {
+        key: 'value',
+      },
+    })
+
+    expect(body).toBeUndefined()
+  })
+
+  it('should handle json response', async () => {
+    const fetchResponse = new Response('{"key":"value"}', {
+      status: 200,
+      headers: {
+        'Content-Type': 'application/json',
+      },
+    })
+
+    const response = await requestHandler['handleFetchResponse'](fetchResponse)
+
+    expect(response).toEqual({
+      status: 200,
+      headers: new Map<string, string | null>([['content-type', 'application/json']]),
+      data: {
+        key: 'value',
+      },
+      key: 'value',
+    })
+  })
+
+  it('should handle non-json response', async () => {
+    const fetchResponse = new Response('body', {
+      status: 200,
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+    })
+
+    const response = await requestHandler['handleFetchResponse'](fetchResponse)
+
+    expect(response.status).toBe(200)
+    expect(response.headers).toEqual(new Map<string, string | null>([['content-type', 'text/plain']]))
+    expect(response.data).toBeInstanceOf(ArrayBuffer)
+  })
+
+  it('should have ratelimit error when forbidden', async () => {
+    const fetchResponse = new Response('body', {
+      status: 403,
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+    })
+
+    const response = await requestHandler['handleFetchResponse'](fetchResponse)
+
+    expect(response.status).toBe(403)
+    expect(response.headers).toEqual(new Map<string, string | null>([['content-type', 'text/plain']]))
+    expect((response.data as HttpErrorResponseBody).error).toEqual({
+      message: ErrorMessage.RateLimited,
+    })
+  })
+
+  describe('should return ErrorResponse when status is not >=200 and <500', () => {
+    it('should add unknown error message when response has no data', async () => {
+      const fetchResponse = new Response('', {
+        status: 599,
+        headers: {
+          'Content-Type': 'text/plain',
+        },
+      })
+
+      const response = await requestHandler['handleFetchResponse'](fetchResponse)
+
+      expect(response.status).toBe(599)
+      expect(response.headers).toEqual(new Map<string, string | null>([['content-type', 'text/plain']]))
+      expect((response.data as HttpErrorResponseBody).error).toEqual({
+        message: 'Unknown error',
+      })
+    })
+  })
+})

packages/api/src/Domain/Http/FetchRequestHandler.ts

@@ -0,0 +1,178 @@
+import {
+  HttpErrorResponse,
+  HttpRequest,
+  HttpRequestParams,
+  HttpResponse,
+  HttpStatusCode,
+  HttpVerb,
+  isErrorResponse,
+} from '@standardnotes/responses'
+import { RequestHandlerInterface } from './RequestHandlerInterface'
+import { Environment } from '@standardnotes/models'
+import { isString } from 'lodash'
+import { ErrorMessage } from '../Error'
+
+export class FetchRequestHandler implements RequestHandlerInterface {
+  constructor(
+    protected readonly snjsVersion: string,
+    protected readonly appVersion: string,
+    protected readonly environment: Environment,
+  ) {}
+
+  async handleRequest<T>(httpRequest: HttpRequest): Promise<HttpResponse<T>> {
+    const request = this.createRequest(httpRequest)
+
+    const response = await this.runRequest<T>(request, this.createRequestBody(httpRequest))
+
+    return response
+  }
+
+  private createRequest(httpRequest: HttpRequest): Request {
+    if (httpRequest.params && httpRequest.verb === HttpVerb.Get && Object.keys(httpRequest.params).length > 0) {
+      httpRequest.url = this.urlForUrlAndParams(httpRequest.url, httpRequest.params)
+    }
+
+    const headers: Record<string, string> = {}
+
+    if (!httpRequest.external) {
+      headers['X-SNJS-Version'] = this.snjsVersion
+
+      const appVersionHeaderValue = `${Environment[this.environment]}-${this.appVersion}`
+      headers['X-Application-Version'] = appVersionHeaderValue
+
+      if (httpRequest.authentication) {
+        headers['Authorization'] = 'Bearer ' + httpRequest.authentication
+      }
+    }
+
+    let contentTypeIsSet = false
+    if (httpRequest.customHeaders && httpRequest.customHeaders.length > 0) {
+      httpRequest.customHeaders.forEach(({ key, value }) => {
+        headers[key] = value
+        if (key === 'Content-Type') {
+          contentTypeIsSet = true
+        }
+      })
+    }
+    if (!contentTypeIsSet && !httpRequest.external) {
+      headers['Content-Type'] = 'application/json'
+    }
+
+    return new Request(httpRequest.url, {
+      method: httpRequest.verb,
+      headers,
+    })
+  }
+
+  private async runRequest<T>(request: Request, body?: string | Uint8Array | undefined): Promise<HttpResponse<T>> {
+    const fetchResponse = await fetch(request, {
+      body,
+    })
+
+    const response = await this.handleFetchResponse<T>(fetchResponse)
+
+    return response
+  }
+
+  private async handleFetchResponse<T>(fetchResponse: Response): Promise<HttpResponse<T>> {
+    const httpStatus = fetchResponse.status
+    const response: HttpResponse<T> = {
+      status: httpStatus,
+      headers: new Map<string, string | null>(),
+      data: {} as T,
+    }
+    fetchResponse.headers.forEach((value, key) => {
+      ;(<Map<string, string | null>>response.headers).set(key, value)
+    })
+
+    try {
+      if (httpStatus !== HttpStatusCode.NoContent) {
+        let body
+
+        const contentTypeHeader = response.headers?.get('content-type') || response.headers?.get('Content-Type')
+
+        if (contentTypeHeader?.includes('application/json')) {
+          body = JSON.parse(await fetchResponse.text())
+        } else {
+          body = await fetchResponse.arrayBuffer()
+        }
+        /**
+         * v0 APIs do not have a `data` top-level object. In such cases, mimic
+         * the newer response body style by putting all the top-level
+         * properties inside a `data` object.
+         */
+        if (!body.data) {
+          response.data = body
+        }
+        if (!isString(body)) {
+          Object.assign(response, body)
+        }
+      }
+    } catch (error) {
+      console.error(error)
+    }
+
+    if (httpStatus >= HttpStatusCode.Success && httpStatus < HttpStatusCode.InternalServerError) {
+      if (httpStatus === HttpStatusCode.Forbidden && isErrorResponse(response)) {
+        if (!response.data.error) {
+          response.data.error = {
+            message: ErrorMessage.RateLimited,
+          }
+        } else {
+          response.data.error.message = ErrorMessage.RateLimited
+        }
+      }
+      return response
+    } else {
+      const errorResponse = response as HttpErrorResponse
+      if (!errorResponse.data) {
+        errorResponse.data = {
+          error: {
+            message: 'Unknown error',
+          },
+        }
+      }
+
+      if (isString(errorResponse.data)) {
+        errorResponse.data = {
+          error: {
+            message: errorResponse.data,
+          },
+        }
+      }
+
+      if (!errorResponse.data.error) {
+        errorResponse.data.error = {
+          message: 'Unknown error',
+        }
+      }
+
+      return errorResponse
+    }
+  }
+
+  private urlForUrlAndParams(url: string, params: HttpRequestParams) {
+    const keyValueString = Object.keys(params as Record<string, unknown>)
+      .map((key) => {
+        return key + '=' + encodeURIComponent((params as Record<string, unknown>)[key] as string)
+      })
+      .join('&')
+
+    if (url.includes('?')) {
+      return url + '&' + keyValueString
+    } else {
+      return url + '?' + keyValueString
+    }
+  }
+
+  private createRequestBody(httpRequest: HttpRequest): string | Uint8Array | undefined {
+    if (
+      httpRequest.params !== undefined &&
+      [HttpVerb.Post, HttpVerb.Put, HttpVerb.Patch, HttpVerb.Delete].includes(httpRequest.verb)
+    ) {
+      return JSON.stringify(httpRequest.params)
+    }
+
+    return httpRequest.rawBytes
+  }
+}

packages/api/src/Domain/Http/HttpService.ts

@@ -1,4 +1,4 @@
-import { isString, joinPaths, sleep } from '@standardnotes/utils'
+import { joinPaths, sleep } from '@standardnotes/utils'
 import { Environment } from '@standardnotes/models'
 import { Session, SessionToken } from '@standardnotes/domain-core'
 import {
@@ -8,15 +8,14 @@ import {
   HttpRequest,
   HttpResponse,
   HttpResponseMeta,
-  HttpErrorResponse,
   isErrorResponse,
 } from '@standardnotes/responses'
 import { HttpServiceInterface } from './HttpServiceInterface'
-import { XMLHttpRequestState } from './XMLHttpRequestState'
-import { ErrorMessage } from '../Error/ErrorMessage'
 
 import { Paths } from '../Server/Auth/Paths'
 import { SessionRefreshResponseBody } from '../Response/Auth/SessionRefreshResponseBody'
+import { FetchRequestHandler } from './FetchRequestHandler'
+import { RequestHandlerInterface } from './RequestHandlerInterface'
 
 export class HttpService implements HttpServiceInterface {
   private session: Session | null
@@ -27,8 +26,11 @@ export class HttpService implements HttpServiceInterface {
   private updateMetaCallback!: (meta: HttpResponseMeta) => void
   private refreshSessionCallback!: (session: Session) => void
 
+  private requestHandler: RequestHandlerInterface
+
   constructor(private environment: Environment, private appVersion: string, private snjsVersion: string) {
     this.session = null
+    this.requestHandler = new FetchRequestHandler(this.snjsVersion, this.appVersion, this.environment)
   }
 
   setCallbacks(
@@ -131,9 +133,7 @@ export class HttpService implements HttpServiceInterface {
       httpRequest.authentication = this.session?.accessToken.value
     }
 
-    const request = this.createXmlRequest(httpRequest)
+    const response = await this.requestHandler.handleRequest<T>(httpRequest)
-
-    const response = await this.runRequest<T>(request, this.createRequestBody(httpRequest))
 
     if (response.meta && !httpRequest.external) {
       this.updateMetaCallback?.(response.meta)
@@ -209,161 +209,4 @@ export class HttpService implements HttpServiceInterface {
 
     return true
   }
-
-  private createRequestBody(httpRequest: HttpRequest): string | Uint8Array | undefined {
-    if (
-      httpRequest.params !== undefined &&
-      [HttpVerb.Post, HttpVerb.Put, HttpVerb.Patch, HttpVerb.Delete].includes(httpRequest.verb)
-    ) {
-      return JSON.stringify(httpRequest.params)
-    }
-
-    return httpRequest.rawBytes
-  }
-
-  private createXmlRequest(httpRequest: HttpRequest) {
-    const request = new XMLHttpRequest()
-    if (httpRequest.params && httpRequest.verb === HttpVerb.Get && Object.keys(httpRequest.params).length > 0) {
-      httpRequest.url = this.urlForUrlAndParams(httpRequest.url, httpRequest.params)
-    }
-    request.open(httpRequest.verb, httpRequest.url, true)
-    request.responseType = httpRequest.responseType ?? ''
-
-    if (!httpRequest.external) {
-      request.setRequestHeader('X-SNJS-Version', this.snjsVersion)
-
-      const appVersionHeaderValue = `${Environment[this.environment]}-${this.appVersion}`
-      request.setRequestHeader('X-Application-Version', appVersionHeaderValue)
-
-      if (httpRequest.authentication) {
-        request.setRequestHeader('Authorization', 'Bearer ' + httpRequest.authentication)
-      }
-    }
-
-    let contenTypeIsSet = false
-    if (httpRequest.customHeaders && httpRequest.customHeaders.length > 0) {
-      httpRequest.customHeaders.forEach(({ key, value }) => {
-        request.setRequestHeader(key, value)
-        if (key === 'Content-Type') {
-          contenTypeIsSet = true
-        }
-      })
-    }
-    if (!contenTypeIsSet && !httpRequest.external) {
-      request.setRequestHeader('Content-Type', 'application/json')
-    }
-
-    return request
-  }
-
-  private async runRequest<T>(request: XMLHttpRequest, body?: string | Uint8Array): Promise<HttpResponse<T>> {
-    return new Promise((resolve) => {
-      request.onreadystatechange = () => {
-        this.stateChangeHandlerForRequest(request, resolve)
-      }
-      request.send(body)
-    })
-  }
-
-  private stateChangeHandlerForRequest<T>(request: XMLHttpRequest, resolve: (response: HttpResponse<T>) => void) {
-    if (request.readyState !== XMLHttpRequestState.Completed) {
-      return
-    }
-    const httpStatus = request.status
-    const response: HttpResponse<T> = {
-      status: httpStatus,
-      headers: new Map<string, string | null>(),
-      data: {} as T,
-    }
-
-    const responseHeaderLines = request
-      .getAllResponseHeaders()
-      ?.trim()
-      .split(/[\r\n]+/)
-    responseHeaderLines?.forEach((responseHeaderLine) => {
-      const parts = responseHeaderLine.split(': ')
-      const name = parts.shift() as string
-      const value = parts.join(': ')
-
-      ;(<Map<string, string | null>>response.headers).set(name, value)
-    })
-
-    try {
-      if (httpStatus !== HttpStatusCode.NoContent) {
-        let body
-
-        const contentTypeHeader = response.headers?.get('content-type') || response.headers?.get('Content-Type')
-
-        if (contentTypeHeader?.includes('application/json')) {
-          body = JSON.parse(request.responseText)
-        } else {
-          body = request.response
-        }
-        /**
-         * v0 APIs do not have a `data` top-level object. In such cases, mimic
-         * the newer response body style by putting all the top-level
-         * properties inside a `data` object.
-         */
-        if (!body.data) {
-          response.data = body
-        }
-        if (!isString(body)) {
-          Object.assign(response, body)
-        }
-      }
-    } catch (error) {
-      console.error(error)
-    }
-    if (httpStatus >= HttpStatusCode.Success && httpStatus < HttpStatusCode.InternalServerError) {
-      if (httpStatus === HttpStatusCode.Forbidden && isErrorResponse(response)) {
-        if (!response.data.error) {
-          response.data.error = {
-            message: ErrorMessage.RateLimited,
-          }
-        } else {
-          response.data.error.message = ErrorMessage.RateLimited
-        }
-      }
-      resolve(response)
-    } else {
-      const errorResponse = response as HttpErrorResponse
-      if (!errorResponse.data) {
-        errorResponse.data = {
-          error: {
-            message: 'Unknown error',
-          },
-        }
-      }
-
-      if (isString(errorResponse.data)) {
-        errorResponse.data = {
-          error: {
-            message: errorResponse.data,
-          },
-        }
-      }
-
-      if (!errorResponse.data.error) {
-        errorResponse.data.error = {
-          message: 'Unknown error',
-        }
-      }
-
-      resolve(errorResponse)
-    }
-  }
-
-  private urlForUrlAndParams(url: string, params: HttpRequestParams) {
-    const keyValueString = Object.keys(params as Record<string, unknown>)
-      .map((key) => {
-        return key + '=' + encodeURIComponent((params as Record<string, unknown>)[key] as string)
-      })
-      .join('&')
-
-    if (url.includes('?')) {
-      return url + '&' + keyValueString
-    } else {
-      return url + '?' + keyValueString
-    }
-  }
 }

packages/api/src/Domain/Http/RequestHandlerInterface.ts

@@ -0,0 +1,5 @@
+import { HttpRequest, HttpResponse } from '@standardnotes/responses'
+
+export interface RequestHandlerInterface {
+  handleRequest<T>(httpRequest: HttpRequest): Promise<HttpResponse<T>>
+}