danwrong / restler

REST client library for node.js
MIT License
1.99k stars 391 forks source link

Compute Content-Leght for UTF8 strings #182

Closed jvverde closed 9 years ago

jvverde commented 9 years ago

I had several errors ECONNRESET and after some research I found this suggestion http://stackoverflow.com/questions/18692580/node-js-post-causes-error-socket-hang-up-code-econnreset

And following it I try to change the following lines

    if (typeof this.options.data == 'string') {
      var buffer = new Buffer(this.options.data, this.options.encoding || 'utf8');
      this.options.data = buffer;
      this.headers['Content-Length'] =  this.options.data.length;
    }

To these ones

    if (typeof this.options.data == 'string') {
      var data = this.options.data;
      var buffer = new Buffer(data, this.options.encoding || 'utf8');
      this.options.data = buffer;
      this.headers['Content-Length'] =  Buffer.byteLength(data, this.options.encoding || 'utf8');
    }

and apparently it solves my problems.

easternbloc commented 9 years ago

I would welcome a PR :)

jvverde commented 9 years ago

PR? What that means? Sorry I don't understand

jvverde commented 9 years ago

oh, I see now: A Pull Request. Sorry I can do that now if you want. Please let me know

easternbloc commented 9 years ago

Ah sorry it stands for Pull Request.

If you can see an issue I'd really appreciate if you can write the fix with tests and pull request it.

easternbloc commented 9 years ago

But please write tests also :)

jvverde commented 9 years ago

Sorry. My mistake. The ECONNRESET error is still there and my proposal doesn't solve it. I see now your code doesn't have any problem as you set the Content-Length as the buffer length and not the string length. Sorry about my mistake. However I still got frequently ECONNRESET when I try to burst several request to the same server (iriscouch.com)

easternbloc commented 9 years ago

No worries.

If you can track down why you're getting the ECONNRESETs please raise another issue cheers.