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

Tcp server did not respond 'Parse Error' #41

Closed
iammapping opened this issue Mar 19, 2015 · 3 comments
Closed

Tcp server did not respond 'Parse Error' #41

iammapping opened this issue Mar 19, 2015 · 3 comments

Comments

@iammapping
Copy link

Hi,
When I wrote to the tcp server with some string not a JSON format, it didn't respond me an 'Parse Error', but the server crashed with 'Error: Unexpected "a" at position 0 in state START'.
The error was thrown from jsonparse module, but unhandled in Utils.parseStream
image
I fix this with the fellowing code, more details in the comment.

Utils.parseStream = function(stream, options, onRequest) {

  var result = JSONStream.parse();

  stream.pipe(result);

  result.on('data', function(data) {

    // apply reviver walk function to prevent stringify/parse again
    if(typeof(options.reviver) === 'function') {
      data = Utils.walk({'': data}, '', options.reviver);
    }

    onRequest(null, data);
  });

  // the jsonparse module always parse until buffer end,
  // because the thrown error has passed to JSONStream stream emit,
  // it never stop while parsing error.
  // e.g. 
  // if buffer is 'abc', it will emit error three times:
  // Unexpected "a" at position 0 in state START
  // Unexpected "b" at position 1 in state START
  // Unexpected "c" at position 2 in state START
  // 
  // add a errored flag to prevent onRequest error from calling more than once
  var errored = false;
  result.on('error', function(err) {
    if (!errored) {
      onRequest(err);
      errored = true;
    }
  });

};

I'm not sure it is graceful, so I just submit a issue not pr. Hope you fix it in a better way.

@tedeh tedeh closed this as completed in c484e90 Mar 23, 2015
@tedeh
Copy link
Owner

tedeh commented Mar 23, 2015

You are absolutely right, errors should be handled when they come from JSONStream. I resolved it in the lastest commit.

@iammapping
Copy link
Author

@tedeh Thank you, this module has saved me much time. Has the fix published in npm?

@tedeh
Copy link
Owner

tedeh commented Mar 24, 2015

Not presently, but I might publish in the coming days.

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