danwrong / restler

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

this.retry crashes application #179

Open iHrushikesh opened 10 years ago

iHrushikesh commented 10 years ago

I am using this.retry as below

rest.put(url)
.on('complete', function(result) {
                    if (result instanceof Error) {
                        log.error(...);
                        this.retry(5000); // try again after 5 sec
                    } else {
                       //process

if the remote REST service is down, the this.retry() keeps calling it at regular interval, eventually causing the host app to crash. Is there any way we can restrict the number of retry attempts?

iHrushikesh commented 10 years ago

I already have a workaround for this issue using a retry count var but hoping for more elegant solution.

YoleYu commented 10 years ago

@iHrushikesh I met the same problem, resolved with the same solution, using a retry count var. Hope retry function can receive a retryTime parameter as input.

ginman86 commented 9 years ago

I have had the same issue. I tracked it down to how the Request object handles encoding the request body. The way it is written, it re-encodes the data every time, growing exponentially. After about 5-6 requests, even a small post body becomes > 1 gb, which is node's default limit.

Current code in /lib/restler.js ~ln 57

} else {
    if (typeof this.options.data == 'object') {
      this.options.data = qs.stringify(this.options.data);
      this.headers['Content-Type'] = 'application/x-www-form-urlencoded';
      this.headers['Content-Length'] = this.options.data.length;
    } 
    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'] = buffer.length;
      }

    if (!this.options.data) {
      this.headers['Content-Length'] = 0;
    }
  }

You can see how additional passes will alternate between being converted into either a string or Buffer, each time being encoded. I.e if it starts as a string, it will be converted into an object. Next round it will be stringified, and then converted back into an object and encoded.

Temp fix:

    if (typeof this.options.data == 'object') {
      if (!(this.options.data instanceof Buffer)) {
          this.options.data = qs.stringify(this.options.data);
      }
      this.headers['Content-Type'] = 'application/x-www-form-urlencoded';
      this.headers['Content-Length'] = this.options.data.length;
    } 
Pawan-Bishnoi commented 6 years ago

Is this fixed now?

this method now includes the said condition.

if (typeof this.options.data == 'object' && !Buffer.isBuffer(this.options.data))