Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silently throws away data from the response if it contains UTF-8 characters #93

Open
StephenWeatherford opened this issue Nov 9, 2017 · 2 comments

Comments

@StephenWeatherford
Copy link

When reading data from an Azure CosmosDB graph, where the data contains non-ansi characters, e.g.:

{
"type": "vertex",
"properties": {
"text": [
{
"id": "a0a0eb9e-8679-4c80-a0e4-ea2e00a96b71",
"value": "RT @colisscom: [JS]このスクリプト一つで、さまざまな用途に合わせたスライダーが実装できて便利 -Tiny Slider\n\nhttps://t.co/ObL6zbEAN4 https://t.co/J0KtrMAgor"
}
],
...

I'm seeing a lot of the data getting silently lost. The problem occurs here in GremlinClient.js:

handleProtocolMessage(message) {
let rawMessage, requestId, statusCode, statusMessage;
try {
const { data } = message;
const buffer = new Buffer(data, 'binary'); <<<< message.data is a string, and message.binary = false
rawMessage = JSON.parse(buffer.toString('utf-8'));
requestId = rawMessage.requestId;
statusCode = rawMessage.status.code;
statusMessage = rawMessage.status.message;
} catch (e) {
this.warn('MalformedResponse', 'Received malformed response message'); <<<< HITS HERE
return;
}

In this case the message is already a UTF-decoded string (message.binary = false). Also, wouldn't it be better to bubble up the exception, rather than silently cutting it off?

Seems like the code should be something like:

  const { data } = message;
  if (message.binary) {
      const buffer = new Buffer(data, 'binary');
      rawMessage = JSON.parse(buffer.toString('utf-8'));
   } else {
      rawMessage = data;
   }
@jbmusso
Copy link
Owner

jbmusso commented Nov 9, 2017 via email

@StephenWeatherford
Copy link
Author

Thanks. I'll submit a PR if I can get some time. I'm temporarily working around it with this code:

// Patch up handleProtocolMessage as a temporary work-around for https://github.com/jbmusso/gremlin-javascript/issues/93
var originalHandleProtocolMessage = client.handleProtocolMessage;
client.handleProtocolMessage = function handleProtocolMessage(message) {
  if (!message.binary) {
    // originalHandleProtocolMessage isn't handling non-binary messages, so convert this one back to binary
    message.data = new Buffer(message.data);
    message.binary = true;
  }

  originalHandleProtocolMessage.call(this, message);
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants