Fallback to a text response if JSON fails (#5)

* Fallback to a text response if JSON fails

* If instead of try-catch

* fetchJSON -> fetchData

* [CI] Add the target branch

* Separate test commands

* Break a test on purpose

* Restore test

* Bump the version

.github/workflows/lint.yml

@@ -6,4 +6,4 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - uses: reviewdog/action-eslint
+      - uses: reviewdog/action-eslint@master

.github/workflows/test.yml

@@ -6,4 +6,11 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - uses: mattallty/jest-github-action
+      - run: npm install
+      - run: npm test
+      - uses: mattallty/jest-github-action@master
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          CI: true
+        with:
+          test-command: 'echo done'

.gitignore

@@ -13,3 +13,4 @@ yalc.lock
 tsconfig.tsbuildinfo
 /openapi
 .#*
+jest.results.json

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@gnosis.pm/safe-react-gateway-sdk",
-  "version": "1.2.0",
+  "version": "1.2.1",
   "main": "dist/index.min.js",
   "types": "dist/index.d.ts",
   "files": [
@@ -39,7 +39,8 @@
     "build": "rm -rf dist && webpack --mode production",
     "prepare": "yarn build",
     "prettier": "prettier -w",
-    "test": "jest tests/"
+    "test:watch": "jest tests/ --watch",
+    "test": "jest --ci --coverage --json --outputFile='jest.results.json' ./tests"
   },
   "husky": {
     "hooks": {

src/endpoint.ts

@@ -1,4 +1,4 @@
-import { fetchJson, insertParams, stringifyQuery } from './utils'
+import { fetchData, insertParams, stringifyQuery } from './utils'
 import { paths } from './types/gateway'
 
 type Primitive = string | number | boolean | null
@@ -25,5 +25,5 @@ export function callEndpoint<T extends keyof paths>(
     body = params?.body
   }
 
-  return fetchJson(url, body)
+  return fetchData(url, body)
 }

src/index.ts

@@ -49,12 +49,12 @@ export function getTransactionQueue(baseUrl: string, address: string, pageUrl?:
   )
 }
 
-export function postTransaction(baseUrl: string, address: string, body: operations['post_transaction']['parameters']['body']) {
-  return callEndpoint(
-    baseUrl,
-    '/transactions/{safe_address}/propose',
-    { path: { safe_address: address }, body },
-  )
+export function postTransaction(
+  baseUrl: string,
+  address: string,
+  body: operations['post_transaction']['parameters']['body'],
+) {
+  return callEndpoint(baseUrl, '/transactions/{safe_address}/propose', { path: { safe_address: address }, body })
 }
 
 /* eslint-enable @typescript-eslint/explicit-module-boundary-types */

src/utils.ts

@@ -29,24 +29,33 @@ export function stringifyQuery(query?: Params): string {
   return searchString ? `?${searchString}` : ''
 }
 
-export function fetchJson<T>(url: string, body?: unknown): Promise<T> {
-  let options: {
-    method: 'POST',
-    headers: Record<string, string>,
-    body: string
-  } | undefined
+export async function fetchData<T>(url: string, body?: unknown): Promise<T> {
+  let options:
+    | {
+        method: 'POST'
+        headers: Record<string, string>
+        body: string
+      }
+    | undefined
   if (body != null) {
     options = {
       method: 'POST',
       body: typeof body === 'string' ? body : JSON.stringify(body),
-      headers: { 'Content-Type': 'application/json' }
+      headers: { 'Content-Type': 'application/json' },
     }
   }
 
-  return fetch(url, options).then((resp) => {
-    if (!resp.ok) {
-      throw Error(resp.statusText)
-    }
-    return resp.json()
-  })
+  const resp = await fetch(url, options)
+
+  if (!resp.ok) {
+    throw Error(resp.statusText)
+  }
+
+  // If the reponse is empty, don't try to parse it
+  const text = await resp.text()
+  if (!text) {
+    return text as unknown as T
+  }
+
+  return resp.json()
 }

tests/endpoint.test.js

@@ -1,75 +1,89 @@
-import fetch from 'unfetch'
+import { fetchData } from '../src/utils'
 import { callEndpoint } from '../src/endpoint'
 
-jest.mock('unfetch', () => jest.fn(() => {
-  return Promise.resolve({
-    ok: true,
-    json: () => Promise.resolve({ success: true })
-  })
-}))
+jest.mock('../src/utils', () => {
+  const originalModule = jest.requireActual('../src/utils')
+
+  return {
+    __esModule: true,
+    ...originalModule,
+    fetchData: jest.fn(() => Promise.resolve({ success: true })),
+  }
+})
 
 describe('callEndpoint', () => {
-  it('should accept just a path', () => {
-    expect(
-      callEndpoint('https://safe-client.xdai.staging.gnosisdev.com/v1', '/balances/supported-fiat-codes')
+  it('should accept just a path', async () => {
+    await expect(
+      callEndpoint('https://safe-client.xdai.staging.gnosisdev.com/v1', '/balances/supported-fiat-codes'),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith('https://safe-client.xdai.staging.gnosisdev.com/v1/balances/supported-fiat-codes', undefined)
+    expect(fetchData).toHaveBeenCalledWith(
+      'https://safe-client.xdai.staging.gnosisdev.com/v1/balances/supported-fiat-codes',
+      undefined,
+    )
   })
 
-  it('should accept a path param', () => {
-    expect(
-      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/safe/{address}', { path: { address: '0x123' } })
+  it('should accept a path param', async () => {
+    await expect(
+      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/safe/{address}', {
+        path: { address: '0x123' },
+      }),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith('https://safe-client.rinkeby.staging.gnosisdev.com/v1/safe/0x123', undefined)
+    expect(fetchData).toHaveBeenCalledWith('https://safe-client.rinkeby.staging.gnosisdev.com/v1/safe/0x123', undefined)
   })
 
-  it('should accept several path params', () => {
-    expect(
-      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/balances/{address}/{currency}', { path: { address: '0x123', currency: 'usd' } })
+  it('should accept several path params', async () => {
+    await expect(
+      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/balances/{address}/{currency}', {
+        path: { address: '0x123', currency: 'usd' },
+      }),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith('https://safe-client.rinkeby.staging.gnosisdev.com/v1/balances/0x123/usd', undefined)
+    expect(fetchData).toHaveBeenCalledWith(
+      'https://safe-client.rinkeby.staging.gnosisdev.com/v1/balances/0x123/usd',
+      undefined,
+    )
   })
 
-  it('should accept query params', () => {
-    expect(
-      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/balances/{address}/{currency}', { path: { address: '0x123', currency: 'usd' }, query: { exclude_spam: true } })
+  it('should accept query params', async () => {
+    await expect(
+      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/balances/{address}/{currency}', {
+        path: { address: '0x123', currency: 'usd' },
+        query: { exclude_spam: true },
+      }),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith('https://safe-client.rinkeby.staging.gnosisdev.com/v1/balances/0x123/usd?exclude_spam=true', undefined)
+    expect(fetchData).toHaveBeenCalledWith(
+      'https://safe-client.rinkeby.staging.gnosisdev.com/v1/balances/0x123/usd?exclude_spam=true',
+      undefined,
+    )
   })
 
-  it('should accept body', () => {
-    expect(
-      callEndpoint(
-        'https://safe-client.rinkeby.staging.gnosisdev.com/v1',
-        '/transactions/{safe_address}/propose',
-        {
-          path: { safe_address: '0x123' },
-          body: { test: 'test' }
-        }
-      )
+  it('should accept body', async () => {
+    await expect(
+      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/transactions/{safe_address}/propose', {
+        path: { safe_address: '0x123' },
+        body: { test: 'test' },
+      }),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith(
+    expect(fetchData).toHaveBeenCalledWith(
       'https://safe-client.rinkeby.staging.gnosisdev.com/v1/transactions/0x123/propose',
-      {
-        method: 'POST',
-        body: '{"test":"test"}',
-        headers: {
-          'Content-Type': 'application/json'
-        }
-      }
+      { test: 'test' },
     )
   })
 
-  it('should accept a raw URL', () => {
-    expect(
-      callEndpoint('https://safe-client.rinkeby.staging.gnosisdev.com/v1', '/balances/{address}/{currency}', { path: { address: '0x123', currency: 'usd' }, query: { exclude_spam: true } }, '/test-url?raw=true')
+  it('should accept a raw URL', async () => {
+    await expect(
+      callEndpoint(
+        'https://safe-client.rinkeby.staging.gnosisdev.com/v1',
+        '/balances/{address}/{currency}',
+        { path: { address: '0x123', currency: 'usd' }, query: { exclude_spam: true } },
+        '/test-url?raw=true',
+      ),
     ).resolves.toEqual({ success: true })
 
-    expect(fetch).toHaveBeenCalledWith('/test-url?raw=true', undefined)
+    expect(fetchData).toHaveBeenCalledWith('/test-url?raw=true', undefined)
   })
 })

tests/utils.test.js

@@ -1,33 +1,25 @@
 import fetch from 'unfetch'
-import { fetchJson, insertParams, stringifyQuery } from '../src/utils'
+import { fetchData, insertParams, stringifyQuery } from '../src/utils'
 
 jest.mock('unfetch')
 
 describe('utils', () => {
   describe('insertParams', () => {
     it('should insert a param into a string', () => {
-      expect(
-        insertParams('/{network}/safe/{address}', { address: '0x0' })
-      ).toBe(
-        '/{network}/safe/0x0'
-      )
+      expect(insertParams('/{network}/safe/{address}', { address: '0x0' })).toBe('/{network}/safe/0x0')
     })
 
     it('should insert several params into a string', () => {
-      expect(
-        insertParams('/{network}/safe/{address}', { address: '0x0', network: 'rinkeby' })
-      ).toBe(
-        '/rinkeby/safe/0x0'
+      expect(insertParams('/{network}/safe/{address}', { address: '0x0', network: 'rinkeby' })).toBe(
+        '/rinkeby/safe/0x0',
       )
     })
   })
 
   describe('stringifyQuery', () => {
     it('should stringify query params', () => {
-      expect(
-        stringifyQuery({ spam: true, page: 11, name: 'token', exclude: null })
-      ).toBe(
-        '?spam=true&page=11&name=token'
+      expect(stringifyQuery({ spam: true, page: 11, name: 'token', exclude: null })).toBe(
+        '?spam=true&page=11&name=token',
       )
     })
 
@@ -38,48 +30,62 @@ describe('utils', () => {
     })
   })
 
-  describe('fetchJson', () => {
-    it('should fetch a simple url', () => {
+  describe('fetchData', () => {
+    it('should fetch a simple url', async () => {
       fetch.mockImplementation(() => {
         return Promise.resolve({
           ok: true,
-          json: () => Promise.resolve({ success: true })
+          text: () => Promise.resolve('{"success": "true"}'),
+          json: () => Promise.resolve({ success: true }),
         })
       })
 
-      expect(fetchJson('/test/safe?q=123')).resolves.toEqual({ success: true })
+      await expect(fetchData('/test/safe?q=123')).resolves.toEqual({ success: true })
       expect(fetch).toHaveBeenCalledWith('/test/safe?q=123', undefined)
     })
 
-    it('should make a post request', () => {
+    it('should make a post request', async () => {
       fetch.mockImplementation(() => {
         return Promise.resolve({
           ok: true,
-          json: () => Promise.resolve({ success: true })
+          text: () => Promise.resolve('{"success": "true"}'),
+          json: () => Promise.resolve({ success: true }),
         })
       })
 
-      expect(fetchJson('/test/safe', '123')).resolves.toEqual({ success: true })
+      await expect(fetchData('/test/safe', '123')).resolves.toEqual({ success: true })
 
       expect(fetch).toHaveBeenCalledWith('/test/safe', {
         method: 'POST',
         body: '123',
         headers: {
-          'Content-Type': 'application/json'
-        }
+          'Content-Type': 'application/json',
+        },
       })
     })
 
-    it('should throw if response is not OK', () => {
+    it('should throw if response is not OK', async () => {
       fetch.mockImplementation(() => {
         return Promise.resolve({
           ok: false,
-          statusText: 'Failed'
+          statusText: 'Failed',
         })
       })
 
-      expect(fetchJson('/test/safe?q=123')).rejects.toThrow('Failed')
+      await expect(fetchData('/test/safe?q=123')).rejects.toThrow('Failed')
       expect(fetch).toHaveBeenCalledWith('/test/safe?q=123', undefined)
     })
+
+    it('should fallback to raw text if no JSON in response', async () => {
+      fetch.mockImplementation(() => {
+        return Promise.resolve({
+          ok: true,
+          text: () => Promise.resolve(''),
+          json: () => Promise.reject('Unexpected end of JSON input'),
+        })
+      })
+
+      await expect(fetchData('/propose', 123)).resolves.toEqual('')
+    })
   })
 })