Fixed no response bug when adding document with invalid xml #14

Closed

jesseclark wants to merge 4 commits from master into master

jesseclark commented

2014-04-03 01:51:12 +01:00

(Migrated from github.com)

When adding/replacing a document containing invalid xml the request would timeout without ever calling the callback method.

In debug mode I could see that the database was returning:
S1<<
'"..." (Line 1): The markup in the document preceding the root element must be well-formed.\u0000\u0001'

This would end up in the else statement in parser2 on line 173 if index.js which would just set the value of self.parser2part without returning a value. This would mean that the parser2 function call would implicitly return undefined which would break out of the while loop in onData (line 106) without ever calling the callback.

This would cause my application hang while waiting for a response and any subsequent calls to the database client to behave unpredictably.

My solution was to return an error response from parser2 with the 'info' field set to 'r.result' so that the callback function would get called from onData with the appropriate parameters.

I added a test case and all tests are passing.

Sorry about reformatting the code a bit. I needed some whitespace in there to read the code more easily.

When adding/replacing a document containing invalid xml the request would timeout without ever calling the callback method. In debug mode I could see that the database was returning: S1<< '"..." (Line 1): The markup in the document preceding the root element must be well-formed.\u0000\u0001' This would end up in the else statement in parser2 on line 173 if index.js which would just set the value of self.parser2part without returning a value. This would mean that the parser2 function call would implicitly return undefined which would break out of the while loop in onData (line 106) without ever calling the callback. This would cause my application hang while waiting for a response and any subsequent calls to the database client to behave unpredictably. My solution was to return an error response from parser2 with the 'info' field set to 'r.result' so that the callback function would get called from onData with the appropriate parameters. I added a test case and all tests are passing. Sorry about reformatting the code a bit. I needed some whitespace in there to read the code more easily.

apb2006 commented

2014-04-03 23:18:30 +01:00

(Migrated from github.com)

Hi Jesse,
Thanks for this. I will merge it. And try to improve the code formating at the same time.

Hi Jesse, Thanks for this. I will merge it. And try to improve the code formating at the same time.

jesseclark commented

2014-04-03 23:23:03 +01:00

(Migrated from github.com)

Thanks, Andy.

On Apr 3, 2014, at 3:18 PM, Andy Bunce notifications@github.com wrote:

Hi Jesse,
Thanks for this. I will merge it. And try to improve the code formating at the same time.

—
Reply to this email directly or view it on GitHub.

Thanks, Andy. On Apr 3, 2014, at 3:18 PM, Andy Bunce notifications@github.com wrote: > Hi Jesse, > Thanks for this. I will merge it. And try to improve the code formating at the same time. > > — > Reply to this email directly or view it on GitHub.

apb2006 commented

2014-04-04 12:34:19 +01:00

(Migrated from github.com)

I believe this is fixed in 0.6.3. The problem was that I was expecting the error response to look like
{partial result} {error} \1 (and had no tests to check that it actually was :-(). In fact it seems it is just {error} \1
http://docs.basex.org/wiki/Server_Protocol
I did not go with your pull as I made a number of other changes, hopefully improvements.
Thanks for reporting it. Streaming is still on my todo list

I believe this is fixed in 0.6.3. The problem was that I was expecting the error response to look like `{partial result} {error} \1` (and had no tests to check that it actually was :-(). In fact it seems it is just `{error} \1` http://docs.basex.org/wiki/Server_Protocol I did not go with your pull as I made a number of other changes, hopefully improvements. Thanks for reporting it. Streaming is still on my todo list

jesseclark commented

2014-04-04 23:29:59 +01:00

(Migrated from github.com)

Ok. I will close the pull request and verify the fix from the main repo.

Pull request closed

Please reopen this pull request to perform a merge.

No reviewers

No labels

dependencies

javascript

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

quodatum/basex-node!14

No description provided.

Rows
Columns