Merge pull request #43 from josemestebandevo/fix/error_handling

Fix API errors handling. Extends not catched errors.

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 Change log.
 
+## 3.1.4
+
+Fix API error processing to avoid calling done callback.
+Catch non catched errors to send them by callbacks as well.
+
 ## 3.1.3
 
 Supports very long records by saving heap memory when downloading data.

lib/fetchStreamReadable/fetchStreamReadable.js

@@ -162,7 +162,6 @@ async function processReader(
         isDone,
         downloaded,
         reader,
-        isErrorParsed(status),
         status,
         callbacks,
         isFirst, // handle first chunk?
@@ -176,6 +175,14 @@ async function processReader(
     } while (!isDone);
   } catch (e) {
     if (e.toString().includes('The user aborted a request.')) console.warn(e);
+    else if (!e.source) {
+      console.error('browser-sdk :: unhandled error :: ', e);
+      callbacks.processError(
+        e instanceof Error ? { error: e.message, type: e.name } :
+          typeof e === 'object' ? e :
+            { error: e }
+      );
+    }
     else console.error('browser-sdk :: ', e);
   } finally {
     cleanPendingEvents(callbacks);
@@ -187,7 +194,6 @@ async function parseChunk(
   done,
   value,
   reader,
-  errorParsed,
   status,
   callbacks,
   isFirst,
@@ -197,16 +203,11 @@ async function parseChunk(
     const timerId = setTimeout(() => {
       // When no more data needs to be consumed
       // or terminal error, break the reading
-      if (done || errorParsed) {
-        measurePerformance('b-sdk-parsing', 'b-sdk-start', 'b-sdk-parsing-end');
-        if (!errorParsed) {
-          callbacks.processDone();
-        }
-        reader.releaseLock();
-      } else {
+      if (value && value.length) {
         try {
           // console.table({bufferString: status.bufferString,
           //remains: status.remains, value: textDecoder.decode(value)});
+          if (done && !value.endsWith(separator)) value += separator;
           status.bufferString += value;
           parse({
             status,
@@ -220,6 +221,14 @@ async function parseChunk(
           clearTimeout(timerId);
         }
       }
+      const errorParsed = isErrorParsed(status);
+      if (done || errorParsed) {
+        measurePerformance('b-sdk-parsing', 'b-sdk-start', 'b-sdk-parsing-end');
+        if (!errorParsed) {
+          callbacks.processDone();
+        }
+        reader.releaseLock();
+      }
       resolve();
     }, 0);
   });

lib/fetchStreamReadable/parser/chunksParser.js

@@ -12,6 +12,7 @@ const {
   isErrorFormat1,
   isErrorFormat2,
   isErrorFormat3,
+  isErrorFormat4,
   joinMsgsIfObjectHasStrings,
 } = require('./parserUtils.js');
 
@@ -50,10 +51,10 @@ function parse({ status, callbacks , isFirst = false, separator }) {
   }
 
   const lastLineIndex = status.bufferString.lastIndexOf(separator);
-  const lasLineIsComplete = lastLineIndex === status.bufferString.length;
+  const lastLineIsComplete = lastLineIndex === status.bufferString.length;
   if(lastLineIndex >= 0){
     const lines = parseBufferStringJson(status, lastLineIndex, separator);
-    if(!lasLineIsComplete){
+    if(!lastLineIsComplete){
       status.bufferString = status.bufferString.substring(lastLineIndex + 1);
     }else{
       status.bufferString = null;
@@ -81,6 +82,7 @@ function parse({ status, callbacks , isFirst = false, separator }) {
           isErrorFormat1(line)
           || isErrorFormat2(line)
           || isErrorFormat3(line)
+          || isErrorFormat4(line)
         ) {
           throw processErrorParsed(callbacks, status, line);
         } else {
@@ -94,6 +96,7 @@ function parse({ status, callbacks , isFirst = false, separator }) {
           logger(`browser-sdk :: chunksParser :: parse :: id = ${status.id} :: errorParsed = ${status.state}:: errorMsg = ${error.msg} ::  bufferString = ${status.bufferString}`);
           const e = new Error();
           e.message = error.msg;
+          e.source = error.source;
           throw e;
         }
       }

lib/fetchStreamReadable/parser/parserUtils.js

@@ -11,6 +11,7 @@ const isErrorFormat2 = (event) =>
   event && (event.msg || event.error) && event.status;
 const isErrorFormat3 = (event) =>
   event && event.e && Array.isArray(event.e) && event.e.length >= 2;
+const isErrorFormat4 = (event) => event && event.e && event.e.msg;
 const joinMsgsIfObjectHasStrings = (event) => {
   if(event && event.object && Array.isArray(event.object)
     && event.object.every(i => (typeof i === 'string'))) {
@@ -47,6 +48,17 @@ function transformErrorFormat3(e) {
   e.object = ['', e.e[1]];
 }
 
+function transformErrorFormat4(e, statusCode) {
+  e.msg = e.e.msg;
+  if (!e.status && statusCode) {
+    e.status = statusCode;
+  }
+  else if (!e.status && e.e) {
+    e.status = e.e.code;
+  }
+  delete e.e;
+}
+
 const parseJson = (s) => {
   try {
     return JSON.parse(s);
@@ -68,8 +80,7 @@ const parseBufferStringJson = (status, lastIndex, separator) => {
 };
 
 /**
- * Normalizes an en error response trying to use
- * the passed error properties.
+ * Normalizes an error response trying to use the passed error properties.
  */
 function createErrorResponse(error) {
   // in some cases error.object is present with 2 items
@@ -81,6 +92,7 @@ function createErrorResponse(error) {
   error.status = 'status' in error ? error.status : 500;
   error.timestamp = 'timestamp' in error ? error.timestamp : 0;
   error.cid = 'cid' in error ? error.cid : '';
+  error.source = 'source' in error ? error.source : 'SDK';
 }
 
 function addToPending(cb, cbName, data) {
@@ -121,6 +133,9 @@ const homogenizeErrorFormat = (errorParsed, statusCode) => {
   } else if (isErrorFormat1(errorParsed)) {
     // same for format 1
     transformErrorFormat1(errorParsed, statusCode);
+  } else if (isErrorFormat4(errorParsed)) {
+    // same for format 4
+    transformErrorFormat4(errorParsed, statusCode);
   }
   if (!errorParsed.status && statusCode) {
     errorParsed.status = statusCode;
@@ -146,6 +161,7 @@ const homogenizeErrorFormat = (errorParsed, statusCode) => {
     errorParsed.object = ['', errorParsed.msg];
   }
 
+  errorParsed.source = 'API';
 };
 
 module.exports = {
@@ -161,5 +177,6 @@ module.exports = {
   isErrorFormat1,
   isErrorFormat2,
   isErrorFormat3,
+  isErrorFormat4,
   joinMsgsIfObjectHasStrings,
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devoinc/browser-sdk",
-  "version": "3.1.3",
+  "version": "3.1.4",
   "description": "Devo browser SDK",
   "author": "Devo Dev Team",
   "eslintConfig": {

test/fetchStreamReadable.test.js

@@ -195,6 +195,11 @@ describe('fetchStreamReadable', () => {
       input.statusCode,
       input.separator || separator
     ).then((result) => {
+      // source is an attribute injected in some errors, but it should not be
+      // in the input (out) so it must be deleted before comparing
+      if (out && out.e && out.e[0].source) delete out.e[0].source;
+      if (outData && outData.e && outData.e[0].source)
+        delete outData.e[0].source;
       JSON.stringify(out).should.be.equal(JSON.stringify(outData));
       result.state.should.be.equal(finalState);
       Object.entries(callbacksExpected).forEach((entry) => {

test/fetchStreamReadable/mocks/errors/errorNdjson.txt

@@ -0,0 +1 @@
+{"e": {"msg": "com.devo.malote.syntax.ParseException: Encountered \" <ID> \"blablabla \"\" at line 1, column 1.\nWas expecting one of:\n    <EOF> \n    \"as\" ...\n    \"every\" ...\n    \"group\" ...\n    \"limit\" ...\n    \"offset\" ...\n    \"order\" ...\n    \"pragma\" ...\n    \"select\" ...\n    \"then\" ...\n    \"where\" ...\n    \",\" ...\n    \";\" ...\n    "}}

test/fetchStreamReadable/mocks/errorsResponses.js

@@ -43,6 +43,8 @@ const accessNotAllowed = toStrFromTxt('./test/fetchStreamReadable/mocks/errors/a
 
 const unauthorizedMessage = toStrFromTxt('./test/fetchStreamReadable/mocks/errors/unauthorizedMessage.txt');
 
+const errorNdjson = toStrFromTxt('./test/fetchStreamReadable/mocks/errors/errorNdjson.txt');
+
 module.exports = {
   genericError1,
   specificError1,
@@ -60,5 +62,6 @@ module.exports = {
   unauthorizedMessage,
   whileExecutingTask,
   wrongFieldName,
-  wrongOperation
+  wrongOperation,
+  errorNdjson,
 };

test/parserUtils.test.js

@@ -15,6 +15,7 @@ describe('parser', () => {
         homogenizeErrorFormat(parsedError, parsedError.status);
         should(parsedError).have.property('msg');
         should(parsedError).have.property('status');
+        should(parsedError).have.property('source');
         createErrorResponse(parsedError);
         should(parsedError).have.property('msg');
         should(parsedError).have.property('status');