Skip to content

Commit 1c96b60

Browse files
noahsilasdplewisacinader
authored
Fix integration tests on later MongoDB (#1113)
When running tests on my computer running Mongodb 4.2.x, this test was failing in a surprising way: ``` Failures: 1) Parse Query can use a regex with all modifiers Message: Uncaught exception: TypeError: message.split is not a function Stack: at <Jasmine> ``` This turns out to be because the error being returned has a `message` attribute that is an object instead of a string. Catching and printing the error shows us an object like this: ``` { Error: [object Object] at handleError (/Users/noahsilas/src/Parse-SDK-JS/lib/node/RESTController.js:380:17) at process._tickCallback (internal/process/next_tick.js:68:7) message: { ok: 0, errmsg: 'Regular expression is invalid: missing )', code: 51091, codeName: 'Location51091', name: 'MongoError' }, code: 1 } ``` Following up on this, we find a [MongoDB issue that sheds some light on this](https://jira.mongodb.org/browse/SERVER-32356): > This begs a few questions. What does the error message "missing )" > mean? Why is the regex valid when specified with quotes but invalid > when specified with slashes? The "missing )" error message I can't > explain: this is fairly cryptic, but it's just the error string that we > have surfaced from the underlying PCRE compilation of the regex. > Regarding the latter question: the difference in behavior between quotes > versus slashes has to do with how the "\n" sequence is interpreted and > the semantics of the "#" character when PCRE_EXTENDED is enabled. > > ... > > There are two interesting facts here. The first is that a newline > character is used together with "#" in order to terminate a comment. > It appears that PCRE requires this terminator, and the regular > expression is considered invalid without one So, what was happening here is that we have a comment in the regex, and that comment needs to be terminated with a newline, or we get very confusing error messaging at every level of the stack. Potential follow ups: - If we see this particular confusing error message, maybe we can surface something nicer to the SDK consumer? - If we get an error object as the response, maybe we can put the object into an attribute other than `message`? Or maybe we should convert it into something like a JSON string and put that into the message field? Mixing types there certainly makes this kind of test failure very opaque. - Maybe we can detect when the 'x' flag is set and we also have an unterminated comment, and if so append the newline automatically in the SDK? This feels a little sketch to me, and potentially confusing if someone is trying to match on a literal '#' character. Co-authored-by: Diamond Lewis <findlewis@gmail.com> Co-authored-by: Arthur Cinader <700572+acinader@users.noreply.github.com>
1 parent a3e9983 commit 1c96b60

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

integration/test/ParseQueryTest.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ describe('Parse Query', () => {
987987
'website',
988988
'parse # First fragment. We\'ll write this in one case but match ' +
989989
'insensitively\n.com # Second fragment. This can be separated by any ' +
990-
'character, including newline',
990+
'character, including newline\n',
991991
'mixs'
992992
);
993993
return query.find();

0 commit comments

Comments
 (0)