Merge branch 'main' into xinrui_DSM_typofix

opensearch-project · Mar 25, 2024 · eb6ecce · eb6ecce
2 parents 3194ed5 + 312075c
commit eb6ecce
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 63 deletions.
diff --git a/.github/workflows/cypress_workflow.yml b/.github/workflows/cypress_workflow.yml
@@ -30,7 +30,7 @@ env:
   TEST_REPO: ${{ inputs.test_repo != '' && inputs.test_repo || 'opensearch-project/opensearch-dashboards-functional-test' }}
   TEST_BRANCH: "${{ inputs.test_branch != '' && inputs.test_branch || github.base_ref }}"
   FTR_PATH: 'ftr'
-  START_CMD: 'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true'
+  START_CMD: 'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true  --csp.warnLegacyBrowsers=false'
   OPENSEARCH_SNAPSHOT_CMD: 'node ../scripts/opensearch snapshot -E cluster.routing.allocation.disk.threshold_enabled=false'
   CYPRESS_BROWSER: 'chromium'
   CYPRESS_VISBUILDER_ENABLED: true

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -77,6 +77,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Discover] Fix lazy loading of the legacy table from getting stuck ([#6041](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6041))
 - [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
 - [osd/std] Add additional recovery from false-positives in handling of long numerals ([#5956](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5956), [#6245](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6245))
+- [osd/std] Add fallback mechanism when recovery from false-positives in handling of long numerals fails ([#6253](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6253))
 - [BUG][Multiple Datasource] Fix missing customApiRegistryPromise param for test connection ([#5944](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5944))
 - [BUG][Multiple Datasource] Add a migration function for datasource to add migrationVersion field ([#6025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6025))
 - [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))

diff --git a/packages/osd-std/src/json.ts b/packages/osd-std/src/json.ts
@@ -143,77 +143,82 @@ const parseStringWithLongNumerals = (
    * To find those instances, we try to parse and watch for the location of any errors. If an error
    * is caused by the marking, we remove that single marking and try again.
    */
-  do {
-    try {
-      hadException = false;
-      obj = parseMarkedText(markedJSON);
-    } catch (e) {
-      hadException = true;
-      /* There are two types of exception objects that can be raised:
-       *  1) a textual message with the position that we need to parse
-       *     i. Unexpected [token|string ...] at position ...
-       *    ii. Expected ',' or ... after ... in JSON at position ...
-       *   iii. expected ',' or ... after ... in object at line ... column ...
-       *  2) a proper object with lineNumber and columnNumber which we can use
-       *    Note: this might refer to the part of the code that threw the exception but
-       *          we will try it anyway; the regex is specific enough to not produce
-       *          false-positives.
-       */
-      let { lineNumber, columnNumber } = e;
-
-      if (typeof e?.message === 'string') {
-        /* Check for 1-i and 1-ii
-         * Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
-         * and the position points to " that is assumed to be the begining of a value.
+  try {
+    do {
+      try {
+        hadException = false;
+        obj = parseMarkedText(markedJSON);
+      } catch (e) {
+        hadException = true;
+        /* There are two types of exception objects that can be raised:
+         *  1) a textual message with the position that we need to parse
+         *     i. Unexpected [token|string ...] at position ...
+         *    ii. Expected ',' or ... after ... in JSON at position ...
+         *   iii. expected ',' or ... after ... in object at line ... column ...
+         *  2) a proper object with lineNumber and columnNumber which we can use
+         *    Note: this might refer to the part of the code that threw the exception but
+         *          we will try it anyway; the regex is specific enough to not produce
+         *          false-positives.
          */
-        let match = e.message.match(/^(?:Une|E)xpected .*at position (\d+)(\D|$)/i);
-        if (match) {
-          lineNumber = 1;
-          // Add 1 to reach the marker
-          columnNumber = parseInt(match[1], 10) + 1;
-        } else {
-          /* Check for 1-iii
-           * Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
-           * and the column number points to the marker after the " that is assumed to be terminating the
-           * value.
-           * PS: There are different versions of this error across browsers and platforms.
+        let { lineNumber, columnNumber } = e;
+
+        if (typeof e?.message === 'string') {
+          /* Check for 1-i and 1-ii
+           * Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
+           * and the position points to " that is assumed to be the begining of a value.
            */
-          // ToDo: Add functional tests for this path
-          match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
+          let match = e.message.match(/^(?:Un)?expected .*at position (\d+)(\D|$)/i);
           if (match) {
-            lineNumber = parseInt(match[1], 10);
-            columnNumber = parseInt(match[2], 10);
+            lineNumber = 1;
+            // Add 1 to reach the marker
+            columnNumber = parseInt(match[1], 10) + 1;
+          } else {
+            /* Check for 1-iii
+             * Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
+             * and the column number points to the marker after the " that is assumed to be terminating the
+             * value.
+             * PS: There are different versions of this error across browsers and platforms.
+             */
+            // ToDo: Add functional tests for this path
+            match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
+            if (match) {
+              lineNumber = parseInt(match[1], 10);
+              columnNumber = parseInt(match[2], 10);
+            }
           }
         }
-      }
 
-      if (lineNumber < 1 || columnNumber < 2) {
-        /* The problem is not with this replacement.
-         * Note: This will never happen because the outer parse would have already thrown.
-         */
-        // coverage:ignore-line
-        throw e;
-      }
+        if (lineNumber < 1 || columnNumber < 2) {
+          /* The problem is not with this replacement.
+           * Note: This will never happen because the outer parse would have already thrown.
+           */
+          // coverage:ignore-line
+          throw e;
+        }
 
-      /* We need to skip e.lineNumber - 1 number of `\n` occurrences.
-       * Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
-       * account for the quote but an additional `-1` is needed because columnNumber starts from 1.
-       */
-      const re = new RegExp(
-        `^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
-      );
-      if (!re.test(markedJSON)) {
-        /* The exception is not caused by adding the marker.
-         * Note: This will never happen because the outer parse would have already thrown.
+        /* We need to skip e.lineNumber - 1 number of `\n` occurrences.
+         * Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
+         * account for the quote but an additional `-1` is needed because columnNumber starts from 1.
          */
-        // coverage:ignore-line
-        throw e;
-      }
+        const re = new RegExp(
+          `^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
+        );
+        if (!re.test(markedJSON)) {
+          /* The exception is not caused by adding the marker.
+           * Note: This will never happen because the outer parse would have already thrown.
+           */
+          // coverage:ignore-line
+          throw e;
+        }
 
-      // We have found a bad replacement; let's remove it.
-      markedJSON = markedJSON.replace(re, '$1$2');
-    }
-  } while (hadException);
+        // We have found a bad replacement; let's remove it.
+        markedJSON = markedJSON.replace(re, '$1$2');
+      }
+    } while (hadException);
+  } catch (ex) {
+    // If parsing of marked `text` fails, fallback to parsing the original `text`
+    obj = JSON.parse(text, reviver || undefined);
+  }
 
   return obj;
 };