Merge remote-tracking branch 'origin/master' into gh-pages

* origin/master:
  v3.2.0
  Response timeout (#1128)
  goto fail syntax
  Complain when .end() is called more than once
  Split .pipe() to avoid calling .end() second time after redirect
  btoa no longer required
  Fire timeout error
  Unify timeout error
  Moved test
  Moved
  Use common test server
  Fixed should no-op
  Use promises to test
  Catch errors in the .end() callback
  Use common server for basic auth
  Use btoa by default only if it's available (#1123)
  Fixed undefined query when sorting without query strings (#1124)

Author: kornelski

History.md

@@ -1,3 +1,12 @@
+# 3.2.0 (2016-12-11)
+
+ * Added `.timeout({response:ms})`, which allows limiting maximum response time independently from total download time (Kornel Lesiński)
+ * Added warnings when `.end()` is called more than once (Kornel Lesiński)
+ * Added `response.links` to browser version (Lukas Eipert)
+ * `btoa` is no longer required in IE9 (Kornel Lesiński)
+ * Fixed `.sortQuery()` on URLs without query strings (Kornel Lesiński)
+ * Refactored common response code into `ResponseBase` (Lukas Eipert)
+
 # 3.1.0 (2016-11-28)
 
  * Added `.sortQuery()` (vicanso)

Readme.md

@@ -35,7 +35,7 @@ Tested browsers:
 - Latest Android, iPhone
 - IE10 through latest. IE9 with polyfills.
 
-Even though IE9 is supported, a polyfill for `window.FormData` is required for `.field()`, and `window.btoa` is needed to use basic auth.
+Even though IE9 is supported, a polyfill for `window.FormData` is required for `.field()`.
 
 Node 4 or later is required.
 

docs/index.md

@@ -353,12 +353,27 @@ For more information, see the Mozilla Developer Network [xhr.responseType docs](
 
   To abort requests simply invoke the `req.abort()` method.
 
-## Request timeouts
+## Timeouts
 
-  A timeout can be applied by invoking `req.timeout(ms)`, after which an error
-  will be triggered. To differentiate between other errors the `err.timeout` property
-  is set to the `ms` value. __NOTE__ that this is a timeout applied to the request
-  and all subsequent redirects, not per request.
+Sometimes networks and servers get "stuck" and never respond after accepting a request. Set timeouts to avoid requests waiting forever.
+
+  * `req.timeout({deadline:ms})` or `req.timeout(ms)` (where `ms` is a number of milliseconds > 0) sets a deadline for the entire request (including all redirects) to complete. If the response isn't fully downloaded within that time, the request will be aborted.
+
+  * `req.timeout({response:ms})` sets maximum time to wait for the first byte to arrive from the server, but it does not limit how long the entire download can take. Response timeout should be a few seconds longer than just the time it takes server to respond, because it also includes time to make DNS lookup, TCP/IP and TLS connections.
+
+You should use both `deadline` and `response` timeouts. This way you can use a short response timeout to detect unresponsive networks quickly, and a long deadline to give time for downloads on slow, but reliable, networks.
+
+    request
+      .get('/big-file?network=slow')
+      .timeout({
+        response: 5000,  // Wait 5 seconds for the server to start sending,
+        deadline: 60000, // but allow 1 minute for the file to finish loading.
+      })
+      .end(function(err, res){
+        if (err.timeout) { /* timed out! */ }
+      });
+
+Timeout errors have a `.timeout` property.
 
 ## Authentication
 

index.html

@@ -265,11 +265,25 @@ <h3 id="response-status">Response status</h3>
  res.forbidden = 403 == status;
 </code></pre><h2 id="aborting-requests">Aborting requests</h2>
 <p>  To abort requests simply invoke the <code>req.abort()</code> method.</p>
-<h2 id="request-timeouts">Request timeouts</h2>
-<p>  A timeout can be applied by invoking <code>req.timeout(ms)</code>, after which an error
-  will be triggered. To differentiate between other errors the <code>err.timeout</code> property
-  is set to the <code>ms</code> value. <strong>NOTE</strong> that this is a timeout applied to the request
-  and all subsequent redirects, not per request.</p>
+<h2 id="timeouts">Timeouts</h2>
+<p>Sometimes networks and servers get &quot;stuck&quot; and never respond after accepting a request. Set timeouts to avoid requests waiting forever.</p>
+<ul>
+<li><p><code>req.timeout({deadline:ms})</code> or <code>req.timeout(ms)</code> (where <code>ms</code> is a number of milliseconds &gt; 0) sets a deadline for the entire request (including all redirects) to complete. If the response isn&#39;t fully downloaded within that time, the request will be aborted.</p>
+</li>
+<li><p><code>req.timeout({response:ms})</code> sets maximum time to wait for the first byte to arrive from the server, but it does not limit how long the entire download can take. Response timeout should be a few seconds longer than just the time it takes server to respond, because it also includes time to make DNS lookup, TCP/IP and TLS connections.</p>
+</li>
+</ul>
+<p>You should use both <code>deadline</code> and <code>response</code> timeouts. This way you can use a short response timeout to detect unresponsive networks quickly, and a long deadline to give time for downloads on slow, but reliable, networks.</p>
+<pre><code>request
+  .get(&#39;/big-file?network=slow&#39;)
+  .timeout({
+    response: 5000,  // Wait 5 seconds for the server to start sending,
+    deadline: 60000, // but allow 1 minute for the file to finish loading.
+  })
+  .end(function(err, res){
+    if (err.timeout) { /* timed out! */ }
+  });
+</code></pre><p>Timeout errors have a <code>.timeout</code> property.</p>
 <h2 id="authentication">Authentication</h2>
 <p>  In both Node and browsers auth available via the <code>.auth()</code> method:</p>
 <pre><code>request

lib/client.js

@@ -521,14 +521,13 @@ Request.prototype.accept = function(type){
 Request.prototype.auth = function(user, pass, options){
   if (!options) {
     options = {
-      type: 'basic'
+      type: 'function' === typeof btoa ? 'basic' : 'auto',
     }
   }
 
   switch (options.type) {
     case 'basic':
-      var str = btoa(user + ':' + pass);
-      this.set('Authorization', 'Basic ' + str);
+      this.set('Authorization', 'Basic ' + btoa(user + ':' + pass));
     break;
 
     case 'auto':
@@ -640,19 +639,6 @@ Request.prototype.pipe = Request.prototype.write = function(){
   throw Error("Streaming is not supported in browser version of superagent");
 };
 
-/**
- * Invoke callback with timeout error.
- *
- * @api private
- */
-
-Request.prototype._timeoutError = function(){
-  var timeout = this._timeout;
-  var err = new Error('timeout of ' + timeout + 'ms exceeded');
-  err.timeout = timeout;
-  this.callback(err);
-};
-
 /**
  * Compose querystring to append to req.url
  *
@@ -662,9 +648,20 @@ Request.prototype._timeoutError = function(){
 Request.prototype._appendQueryString = function(){
   var query = this._query.join('&');
   if (query) {
-    this.url += ~this.url.indexOf('?')
-      ? '&' + query
-      : '?' + query;
+    this.url += (this.url.indexOf('?') >= 0 ? '&' : '?') + query;
+  }
+
+  if (this._sort) {
+    var index = this.url.indexOf('?');
+    if (index >= 0) {
+      var queryArr = this.url.substring(index + 1).split('&');
+      if (isFunction(this._sort)) {
+        queryArr.sort(this._sort);
+      } else {
+        queryArr.sort();
+      }
+      this.url = this.url.substring(0, index) + '?' + queryArr.join('&');
+    }
   }
 };
 
@@ -693,24 +690,33 @@ Request.prototype._isHost = function _isHost(obj) {
 Request.prototype.end = function(fn){
   var self = this;
   var xhr = this.xhr = request.getXHR();
-  var timeout = this._timeout;
   var data = this._formData || this._data;
 
+  if (this._endCalled) {
+    console.warn("Warning: .end() was called twice. This is not supported in superagent");
+  }
+  this._endCalled = true;
+
   // store callback
   this._callback = fn || noop;
 
   // state change
   xhr.onreadystatechange = function(){
-    if (4 != xhr.readyState) return;
+    var readyState = xhr.readyState;
+    if (readyState >= 2 && self._responseTimeoutTimer) {
+      clearTimeout(self._responseTimeoutTimer);
+    }
+    if (4 != readyState) {
+      return;
+    }
 
     // In IE9, reads to any property (e.g. status) off of an aborted XHR will
     // result in the error "Could not complete the operation due to error c00c023f"
     var status;
     try { status = xhr.status } catch(e) { status = 0; }
 
-    if (0 == status) {
-      if (self.timedout) return self._timeoutError();
-      if (self._aborted) return;
+    if (!status) {
+      if (self.timedout || self._aborted) return;
       return self.crossDomainError();
     }
     self.emit('end');
@@ -737,25 +743,10 @@ Request.prototype.end = function(fn){
     }
   }
 
-  // timeout
-  if (timeout && !this._timer) {
-    this._timer = setTimeout(function(){
-      self.timedout = true;
-      self.abort();
-    }, timeout);
-  }
-
   // querystring
   this._appendQueryString();
 
-  if (this._sort) {
-    var index = this.url.indexOf('?');
-    if (~index) {
-      var queryArr = this.url.substring(index + 1).split('&');
-      isFunction(this._sort) ? queryArr.sort(this._sort) : queryArr.sort();
-    }
-    this.url = this.url.substring(0, index) + '?' + queryArr.join('&');
-  }
+  this._setTimeouts();
 
   // initiate request
   if (this.username && this.password) {
@@ -772,7 +763,9 @@ Request.prototype.end = function(fn){
     // serialize stuff
     var contentType = this._header['content-type'];
     var serialize = this._serializer || request.serialize[contentType ? contentType.split(';')[0] : ''];
-    if (!serialize && isJSON(contentType)) serialize = request.serialize['application/json'];
+    if (!serialize && isJSON(contentType)) {
+      serialize = request.serialize['application/json'];
+    }
     if (serialize) data = serialize(data);
   }
 

lib/node/index.js

@@ -345,12 +345,17 @@ Request.prototype.write = function(data, encoding){
 Request.prototype.pipe = function(stream, options){
   this.piped = true; // HACK...
   this.buffer(false);
+  this.end();
+  return this._pipeContinue(stream, options);
+};
+
+Request.prototype._pipeContinue = function(stream, options){
   var self = this;
-  this.end().req.once('response', function(res){
+  this.req.once('response', function(res){
     // redirect
     var redirect = isRedirect(res.statusCode);
     if (redirect && self._redirects++ != self._maxRedirects) {
-      return self._redirect(res).pipe(stream, options);
+      return self._redirect(res)._pipeContinue(stream, options);
     }
 
     self.res = res;
@@ -446,6 +451,7 @@ Request.prototype._redirect = function(res){
   _initHeaders(this)
 
   // redirect
+  this._endCalled = false;
   this.url = url;
   this.qs = {};
   this.qsRaw = [];
@@ -575,7 +581,9 @@ Request.prototype.request = function(){
 
   // request
   var req = this.req = mod.request(options);
-  if ('HEAD' != options.method) req.setHeader('Accept-Encoding', 'gzip, deflate');
+  if ('HEAD' != options.method) {
+    req.setHeader('Accept-Encoding', 'gzip, deflate');
+  }
   this.protocol = url.protocol;
   this.host = url.host;
 
@@ -661,18 +669,21 @@ Request.prototype.callback = function(err, res){
  */
 
 Request.prototype._appendQueryString = function(req){
-  var querystring = qs.stringify(this.qs, { indices: false, strictNullHandling: true });
-  querystring += ((querystring.length && this.qsRaw.length) ? '&' : '') + this.qsRaw.join('&');
-  req.path += querystring.length
-    ? (~req.path.indexOf('?') ? '&' : '?') + querystring
-    : '';
+  var query = qs.stringify(this.qs, { indices: false, strictNullHandling: true });
+  query += ((query.length && this.qsRaw.length) ? '&' : '') + this.qsRaw.join('&');
+  req.path += query.length ? (~req.path.indexOf('?') ? '&' : '?') + query : '';
+
   if (this._sort) {
     var index = req.path.indexOf('?');
-    if (~index) {
+    if (index >= 0) {
       var queryArr = req.path.substring(index + 1).split('&');
-      isFunction(this._sort) ? queryArr.sort(this._sort) : queryArr.sort();
+      if (isFunction(this._sort)) {
+        queryArr.sort(this._sort);
+      } else {
+        queryArr.sort();
+      }
+      req.path = req.path.substring(0, index) + '?' + queryArr.join('&');
     }
-    req.path = req.path.substring(0, index) + '?' + queryArr.join('&');
   }
 };
 
@@ -714,9 +725,13 @@ Request.prototype.end = function(fn){
   var req = this.request();
   var buffer = this._buffer;
   var method = this.method;
-  var timeout = this._timeout;
   debug('%s %s', this.method, this.url);
 
+  if (this._endCalled) {
+    console.warn("Warning: .end() was called twice. This is not supported in superagent");
+  }
+  this._endCalled = true;
+
   // store callback
   this._callback = fn || noop;
 
@@ -727,18 +742,7 @@ Request.prototype.end = function(fn){
     return this.callback(e);
   }
 
-  // timeout
-  if (timeout && !this._timer) {
-    debug('timeout %sms %s %s', timeout, this.method, this.url);
-    this._timer = setTimeout(function(){
-      var err = new Error('timeout of ' + timeout + 'ms exceeded');
-      err.timeout = timeout;
-      err.code = 'ECONNABORTED';
-      self.timedout = true;
-      self.abort();
-      self.callback(err);
-    }, timeout);
-  }
+  this._setTimeouts();
 
   // body
   if ('HEAD' != method && !req._headerSent) {
@@ -748,7 +752,9 @@ Request.prototype.end = function(fn){
       // Parse out just the content type from the header (ignore the charset)
       if (contentType) contentType = contentType.split(';')[0]
       var serialize = exports.serialize[contentType];
-      if (!serialize && isJSON(contentType)) serialize = exports.serialize['application/json'];
+      if (!serialize && isJSON(contentType)) {
+        serialize = exports.serialize['application/json'];
+      }
       if (serialize) data = serialize(data);
     }
 
@@ -762,6 +768,10 @@ Request.prototype.end = function(fn){
   req.once('response', function(res){
     debug('%s %s -> %s', self.method, self.url, res.statusCode);
 
+    if (self._responseTimeoutTimer) {
+      clearTimeout(self._responseTimeoutTimer);
+    }
+
     if (self.piped) {
       return;
     }
@@ -815,7 +825,9 @@ Request.prototype.end = function(fn){
     }
 
     // by default only buffer text/*, json and messed up thing from hell
-    if (undefined === buffer && isText(mime) || isJSON(mime)) buffer = true;
+    if (undefined === buffer && isText(mime) || isJSON(mime)) {
+      buffer = true;
+    }
 
     var parserHandlesEnd = false;
     if (parser) {