Merge pull request #209 from influxdata/208/1.8_error

fix(core/transport): fall back to 1.8 X-Influxdb-Error HTTP header when no error body is available
influxdata · Jun 30, 2020 · c79167c · c79167c
2 parents 01ed83f + b2271cf
commit c79167c
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### Features
 
 1. [#204](https://github.com/influxdata/influxdb-client-js/pull/204): Allow to supply default tags in WriteOptions.
+1. [#209](https://github.com/influxdata/influxdb-client-js/pull/209): Better report errors when InfluxDB 1.8 returns empty body
 
 ### Documentation
 

diff --git a/packages/core/src/impl/browser/FetchTransport.ts b/packages/core/src/impl/browser/FetchTransport.ts
@@ -67,6 +67,12 @@ export default class FetchTransport implements Transport {
           return response
             .text()
             .then((text: string) => {
+              if (!text) {
+                const headerError = response.headers.get('x-influxdb-error')
+                if (headerError) {
+                  text = headerError
+                }
+              }
               observer.error(
                 new HttpError(
                   response.status,
@@ -124,6 +130,12 @@ export default class FetchTransport implements Transport {
       Logger.warn('Unable to read error body', _e)
     }
     if (status >= 300) {
+      if (!data) {
+        const headerError = headers.get('x-influxdb-error')
+        if (headerError) {
+          data = headerError
+        }
+      }
       throw new HttpError(
         status,
         response.statusText,

diff --git a/packages/core/src/impl/node/NodeHttpTransport.ts b/packages/core/src/impl/node/NodeHttpTransport.ts
@@ -214,7 +214,10 @@ export class NodeHttpTransport implements Transport {
             res.resume()
           }
         })
-        responseData.on('end', () =>
+        responseData.on('end', () => {
+          if (body === '' && !!res.headers['x-influxdb-error']) {
+            body = res.headers['x-influxdb-error'].toString()
+          }
           listeners.error(
             new HttpError(
               statusCode,
@@ -223,7 +226,7 @@ export class NodeHttpTransport implements Transport {
               res.headers['retry-after']
             )
           )
-        )
+        })
       } else {
         responseData.on('data', data => {
           if (cancellable.isCancelled()) {

diff --git a/packages/core/test/unit/impl/browser/FetchTransport.test.ts b/packages/core/test/unit/impl/browser/FetchTransport.test.ts
@@ -97,6 +97,44 @@ describe('FetchTransport', () => {
         // console.log(`        OK, received ${_e}`)
       }
     })
+    it('throws error with X-Influxdb-Error header body', async () => {
+      const message = 'this is a header message'
+      emulateFetchApi({
+        headers: {
+          'content-type': 'application/json',
+          'x-influxdb-error': message,
+        },
+        body: '',
+        status: 500,
+      })
+      try {
+        await transport.request('/whatever', '', {
+          method: 'GET',
+        })
+        expect.fail()
+      } catch (e) {
+        expect(e)
+          .property('body')
+          .equals(message)
+      }
+    })
+    it('throws error with empty body', async () => {
+      emulateFetchApi({
+        headers: {'content-type': 'text/plain'},
+        body: '',
+        status: 500,
+      })
+      try {
+        await transport.request('/whatever', '', {
+          method: 'GET',
+        })
+        expect.fail()
+      } catch (e) {
+        expect(e)
+          .property('body')
+          .equals('')
+      }
+    })
   })
   describe('send', () => {
     const transport = new FetchTransport({url: 'http://test:9999'})
@@ -141,61 +179,94 @@ describe('FetchTransport', () => {
         callbacks: fakeCallbacks(),
         status: 501,
       },
-    ].forEach(({body, callbacks, url = '/whatever', status = 200}, i) => {
-      it(`receives data in chunks ${i}`, async () => {
-        emulateFetchApi({
-          headers: {'content-type': 'text/plain', duplicate: 'ok'},
-          status,
+      {
+        body: '',
+        callbacks: fakeCallbacks(),
+        status: 500,
+        headers: {'x-influxdb-error': 'header error'},
+        errorBody: 'header error',
+      },
+      {
+        body: '',
+        callbacks: fakeCallbacks(),
+        status: 500,
+        errorBody: '',
+      },
+    ].forEach(
+      (
+        {
           body,
-        })
-        if (callbacks) {
-          await new Promise((resolve: any) =>
-            transport.send(
-              url,
-              '',
-              {method: 'POST'},
-              {
-                ...callbacks,
-                complete() {
-                  callbacks.complete && callbacks.complete()
-                  resolve()
-                },
-                error(e: Error) {
-                  callbacks.error && callbacks.error(e)
-                  resolve()
-                },
-              }
+          callbacks,
+          url = '/whatever',
+          status = 200,
+          headers = {},
+          errorBody,
+        },
+        i
+      ) => {
+        it(`receives data in chunks ${i}`, async () => {
+          emulateFetchApi({
+            headers: {
+              'content-type': 'text/plain',
+              duplicate: 'ok',
+              ...headers,
+            },
+            status,
+            body,
+          })
+          if (callbacks) {
+            await new Promise((resolve: any) =>
+              transport.send(
+                url,
+                '',
+                {method: 'POST'},
+                {
+                  ...callbacks,
+                  complete() {
+                    callbacks.complete && callbacks.complete()
+                    resolve()
+                  },
+                  error(e: Error) {
+                    callbacks.error && callbacks.error(e)
+                    resolve()
+                  },
+                }
+              )
             )
-          )
-          if (callbacks.useCancellable) {
-            expect(callbacks.useCancellable.callCount).equals(1)
-            const cancellable = callbacks.useCancellable.args[0][0]
-            cancellable.cancel()
-            expect(cancellable.isCancelled()).is.equal(true)
-          }
-          const isError = url === 'error' || status !== 200
-          expect(callbacks.responseStarted.callCount).equals(
-            url === 'error' ? 0 : 1
-          )
-          expect(callbacks.complete.callCount).equals(isError ? 0 : 1)
-          expect(callbacks.error.callCount).equals(isError ? 1 : 0)
-          expect(callbacks.next.callCount).equals(
-            isError ? 0 : Array.isArray(body) ? body.length : 1
-          )
-          if (!isError) {
-            const vals = callbacks.next.args.map((args: any) =>
-              Buffer.from(args[0])
+            if (callbacks.useCancellable) {
+              expect(callbacks.useCancellable.callCount).equals(1)
+              const cancellable = callbacks.useCancellable.args[0][0]
+              cancellable.cancel()
+              expect(cancellable.isCancelled()).is.equal(true)
+            }
+            const isError = url === 'error' || status !== 200
+            expect(callbacks.responseStarted.callCount).equals(
+              url === 'error' ? 0 : 1
             )
-            expect(
-              Array.isArray(body)
-                ? body
-                : [Buffer.isBuffer(body) ? body : Buffer.from(body)]
-            ).is.deep.equal(vals)
+            expect(callbacks.complete.callCount).equals(isError ? 0 : 1)
+            expect(callbacks.error.callCount).equals(isError ? 1 : 0)
+            expect(callbacks.next.callCount).equals(
+              isError ? 0 : Array.isArray(body) ? body.length : 1
+            )
+            if (!isError) {
+              const vals = callbacks.next.args.map((args: any) =>
+                Buffer.from(args[0])
+              )
+              expect(
+                Array.isArray(body)
+                  ? body
+                  : [Buffer.isBuffer(body) ? body : Buffer.from(body)]
+              ).is.deep.equal(vals)
+            } else if (errorBody) {
+              expect(callbacks.error.args[0][0])
+                .property('body')
+                .equals(errorBody)
+            }
+          } else {
+            transport.send('/whatever', '', {method: 'POST'}, callbacks)
           }
-        } else {
-          transport.send('/whatever', '', {method: 'POST'}, callbacks)
-        }
-      })
-    })
+        })
+      }
+    )
   })
 })
diff --git a/packages/core/test/unit/impl/browser/emulateBrowser.ts b/packages/core/test/unit/impl/browser/emulateBrowser.ts
@@ -28,8 +28,10 @@ function createResponse({
     },
     json(): Promise<any> {
       if (typeof body === 'string') {
-        if (body == 'error') return Promise.reject(new Error('error data'))
-        return Promise.resolve(JSON.parse(body))
+        if (body === 'error') return Promise.reject(new Error('error data'))
+        return Promise.resolve(body).then(body =>
+          body ? JSON.parse(body) : ''
+        )
       } else {
         return Promise.reject(new Error('String body expected, but ' + body))
       }
@@ -38,7 +40,7 @@ function createResponse({
   if (typeof body === 'string') {
     retVal.text = function(): Promise<string> {
       if (typeof body === 'string') {
-        if (body == 'error') return Promise.reject(new Error('error data'))
+        if (body === 'error') return Promise.reject(new Error('error data'))
         return Promise.resolve(body)
       } else {
         return Promise.reject(new Error('String body expected, but ' + body))

diff --git a/packages/core/test/unit/impl/node/NodeHttpTransport.test.ts b/packages/core/test/unit/impl/node/NodeHttpTransport.test.ts
@@ -300,6 +300,21 @@ describe('NodeHttpTransport', () => {
               .to.length(1000)
           })
       })
+      it(`uses X-Influxdb-Error header when no body is returned`, async () => {
+        const errorMessage = 'this is a header error message'
+        nock(transportOptions.url)
+          .get('/test')
+          .reply(500, '', {'X-Influxdb-Error': errorMessage})
+        await sendTestData(transportOptions, {method: 'GET'})
+          .then(() => {
+            throw new Error('must not succeed')
+          })
+          .catch((e: any) => {
+            expect(e)
+              .property('body')
+              .equals(errorMessage)
+          })
+      })
       it(`is aborted before the whole response arrives`, async () => {
         let remainingChunks = 2
         let res: any