Avoid emitting aborted errors into the response

serviejs · Jul 8, 2016 · 8a77d01 · 8a77d01
1 parent 45ee832
commit 8a77d01
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 22 deletions.
diff --git a/lib/browser.ts b/lib/browser.ts
@@ -21,7 +21,7 @@ function open (request: Request) {
 
     const xhr = request._raw = new XMLHttpRequest()
 
-    xhr.onload = function () {
+    function done () {
       return resolve({
         status: xhr.status === 1223 ? 204 : xhr.status,
         statusText: xhr.statusText,
@@ -31,9 +31,8 @@ function open (request: Request) {
       })
     }
 
-    xhr.onabort = function () {
-      return reject(request.error('Request aborted', 'EABORT'))
-    }
+    xhr.onload = done
+    xhr.onabort = done
 
     xhr.onerror = function () {
       return reject(request.error(`Unable to connect to "${request.url}"`, 'EUNAVAILABLE'))
@@ -104,14 +103,16 @@ function abort (request: Request) {
  */
 function parseToRawHeaders (headers: string): RawHeaders {
   const rawHeaders: RawHeaders = []
-  const lines = headers.replace(/\r?\n$/, '').split(/\r?\n/)
+  const lines = headers.split(/\r?\n/)
 
-  for (const header of lines) {
-    const indexOf = header.indexOf(':')
-    const name = header.substr(0, indexOf).trim()
-    const value = header.substr(indexOf + 1).trim()
+  for (const line of lines) {
+    if (line) {
+      const indexOf = line.indexOf(':')
+      const name = line.substr(0, indexOf).trim()
+      const value = line.substr(indexOf + 1).trim()
 
-    rawHeaders.push(name, value)
+      rawHeaders.push(name, value)
+    }
   }
 
   return rawHeaders

diff --git a/lib/index.ts b/lib/index.ts
@@ -170,10 +170,6 @@ function open (request: Request) {
             emitError(request.error(`Unable to connect to "${url}"`, 'EUNAVAILABLE', error))
           })
 
-          rawRequest.once('clientAborted', function () {
-            emitError(request.error('Request aborted', 'EABORT'))
-          })
-
           request._raw = rawRequest
           request.uploadLength = num(rawRequest.getHeader('content-length'), 0)
           requestStream.pipe(rawRequest)
@@ -201,7 +197,6 @@ function open (request: Request) {
  * Close the current HTTP request.
  */
 function abort (request: Request) {
-  request._raw.emit('clientAborted')
   request._raw.abort()
 }
 

diff --git a/lib/request.ts b/lib/request.ts
@@ -170,7 +170,6 @@ export default class Request extends Base implements Promise<Response> {
 
     // Abort the current handler.
     this.aborted = true
-    this._reject(this.error('Request aborted', 'EABORT'))
 
     // Sometimes it's just not possible to abort.
     if (this.opened) {
@@ -182,6 +181,9 @@ export default class Request extends Base implements Promise<Response> {
       }
     }
 
+    // Reject _after_ the transport handles abort resolution.
+    this._reject(this.error('Request aborted', 'EABORT'))
+
     return this
   }
 

diff --git a/lib/test/index.ts b/lib/test/index.ts
@@ -425,17 +425,23 @@ test('abort', function (t) {
   t.test('abort mid-request', function (t) {
     const req = popsicle.request(REMOTE_URL + '/download')
 
-    t.plan(3)
+    t.plan(1)
 
     setTimeout(function () {
       req.abort()
     }, 100)
 
+    // Browser requests can not be aborted mid-request.
+    if (popsicle.browser) {
+      return req
+        .catch(function (err) {
+          t.equal(err.code, 'EABORT')
+        })
+    }
+
     return req
-      .catch(function (err) {
-        t.equal(err.message, 'Request aborted')
-        t.equal(err.code, 'EABORT')
-        t.ok(err.popsicle instanceof popsicle.Request)
+      .then(function (res) {
+        t.equal(res.get('Content-Length'), '12')
       })
   })
 
@@ -1026,4 +1032,4 @@ if (!popsicle.browser) {
         t.equal(res.body, 'Success')
       })
   })
-}
+}