Skip to content

Commit 312075c

Browse files
authored
[osd/std] Add fallback mechanism when recovery from false-positives in handling of long numerals fails (#6253)
Signed-off-by: Miki <[email protected]>
1 parent 27d73ab commit 312075c

File tree

2 files changed

+68
-62
lines changed

2 files changed

+68
-62
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
7676
- [Discover] Fix lazy loading of the legacy table from getting stuck ([#6041](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6041))
7777
- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
7878
- [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))
79+
- [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))
7980
- [BUG][Multiple Datasource] Fix missing customApiRegistryPromise param for test connection ([#5944](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5944))
8081
- [BUG][Multiple Datasource] Add a migration function for datasource to add migrationVersion field ([#6025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6025))
8182
- [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))

packages/osd-std/src/json.ts

+67-62
Original file line numberDiff line numberDiff line change
@@ -143,77 +143,82 @@ const parseStringWithLongNumerals = (
143143
* To find those instances, we try to parse and watch for the location of any errors. If an error
144144
* is caused by the marking, we remove that single marking and try again.
145145
*/
146-
do {
147-
try {
148-
hadException = false;
149-
obj = parseMarkedText(markedJSON);
150-
} catch (e) {
151-
hadException = true;
152-
/* There are two types of exception objects that can be raised:
153-
* 1) a textual message with the position that we need to parse
154-
* i. Unexpected [token|string ...] at position ...
155-
* ii. Expected ',' or ... after ... in JSON at position ...
156-
* iii. expected ',' or ... after ... in object at line ... column ...
157-
* 2) a proper object with lineNumber and columnNumber which we can use
158-
* Note: this might refer to the part of the code that threw the exception but
159-
* we will try it anyway; the regex is specific enough to not produce
160-
* false-positives.
161-
*/
162-
let { lineNumber, columnNumber } = e;
163-
164-
if (typeof e?.message === 'string') {
165-
/* Check for 1-i and 1-ii
166-
* Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
167-
* and the position points to " that is assumed to be the begining of a value.
146+
try {
147+
do {
148+
try {
149+
hadException = false;
150+
obj = parseMarkedText(markedJSON);
151+
} catch (e) {
152+
hadException = true;
153+
/* There are two types of exception objects that can be raised:
154+
* 1) a textual message with the position that we need to parse
155+
* i. Unexpected [token|string ...] at position ...
156+
* ii. Expected ',' or ... after ... in JSON at position ...
157+
* iii. expected ',' or ... after ... in object at line ... column ...
158+
* 2) a proper object with lineNumber and columnNumber which we can use
159+
* Note: this might refer to the part of the code that threw the exception but
160+
* we will try it anyway; the regex is specific enough to not produce
161+
* false-positives.
168162
*/
169-
let match = e.message.match(/^(?:Une|E)xpected .*at position (\d+)(\D|$)/i);
170-
if (match) {
171-
lineNumber = 1;
172-
// Add 1 to reach the marker
173-
columnNumber = parseInt(match[1], 10) + 1;
174-
} else {
175-
/* Check for 1-iii
176-
* Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
177-
* and the column number points to the marker after the " that is assumed to be terminating the
178-
* value.
179-
* PS: There are different versions of this error across browsers and platforms.
163+
let { lineNumber, columnNumber } = e;
164+
165+
if (typeof e?.message === 'string') {
166+
/* Check for 1-i and 1-ii
167+
* Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
168+
* and the position points to " that is assumed to be the begining of a value.
180169
*/
181-
// ToDo: Add functional tests for this path
182-
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
170+
let match = e.message.match(/^(?:Un)?expected .*at position (\d+)(\D|$)/i);
183171
if (match) {
184-
lineNumber = parseInt(match[1], 10);
185-
columnNumber = parseInt(match[2], 10);
172+
lineNumber = 1;
173+
// Add 1 to reach the marker
174+
columnNumber = parseInt(match[1], 10) + 1;
175+
} else {
176+
/* Check for 1-iii
177+
* Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
178+
* and the column number points to the marker after the " that is assumed to be terminating the
179+
* value.
180+
* PS: There are different versions of this error across browsers and platforms.
181+
*/
182+
// ToDo: Add functional tests for this path
183+
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
184+
if (match) {
185+
lineNumber = parseInt(match[1], 10);
186+
columnNumber = parseInt(match[2], 10);
187+
}
186188
}
187189
}
188-
}
189190

190-
if (lineNumber < 1 || columnNumber < 2) {
191-
/* The problem is not with this replacement.
192-
* Note: This will never happen because the outer parse would have already thrown.
193-
*/
194-
// coverage:ignore-line
195-
throw e;
196-
}
191+
if (lineNumber < 1 || columnNumber < 2) {
192+
/* The problem is not with this replacement.
193+
* Note: This will never happen because the outer parse would have already thrown.
194+
*/
195+
// coverage:ignore-line
196+
throw e;
197+
}
197198

198-
/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
199-
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
200-
* account for the quote but an additional `-1` is needed because columnNumber starts from 1.
201-
*/
202-
const re = new RegExp(
203-
`^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
204-
);
205-
if (!re.test(markedJSON)) {
206-
/* The exception is not caused by adding the marker.
207-
* Note: This will never happen because the outer parse would have already thrown.
199+
/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
200+
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
201+
* account for the quote but an additional `-1` is needed because columnNumber starts from 1.
208202
*/
209-
// coverage:ignore-line
210-
throw e;
211-
}
203+
const re = new RegExp(
204+
`^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
205+
);
206+
if (!re.test(markedJSON)) {
207+
/* The exception is not caused by adding the marker.
208+
* Note: This will never happen because the outer parse would have already thrown.
209+
*/
210+
// coverage:ignore-line
211+
throw e;
212+
}
212213

213-
// We have found a bad replacement; let's remove it.
214-
markedJSON = markedJSON.replace(re, '$1$2');
215-
}
216-
} while (hadException);
214+
// We have found a bad replacement; let's remove it.
215+
markedJSON = markedJSON.replace(re, '$1$2');
216+
}
217+
} while (hadException);
218+
} catch (ex) {
219+
// If parsing of marked `text` fails, fallback to parsing the original `text`
220+
obj = JSON.parse(text, reviver || undefined);
221+
}
217222

218223
return obj;
219224
};

0 commit comments

Comments
 (0)