request: don't return error as String

This bug has been lurking for a really long time, but was only recently
exposed by changes to the registry architecture that make it more common
to return a 404 without a body.

Fixes: #112

Author: othiym23

lib/request.js

@@ -9,6 +9,7 @@ var assert = require('assert')
 var url = require('url')
 var zlib = require('zlib')
 var Stream = require('stream').Stream
+var STATUS_CODES = require('http').STATUS_CODES
 
 var request = require('request')
 var once = require('once')
@@ -208,8 +209,9 @@ function requestDone (method, where, cb) {
 
     // expect data with any error codes
     if (!data && response.statusCode >= 400) {
+      var code = response.statusCode
       return cb(
-        response.statusCode + ' ' + require('http').STATUS_CODES[response.statusCode],
+        new Error(code + ' ' + STATUS_CODES[code]),
         null,
         data,
         response

test/fetch-404.js

@@ -1,9 +1,7 @@
 var resolve = require('path').resolve
 var createReadStream = require('graceful-fs').createReadStream
-var readFileSync = require('graceful-fs').readFileSync
 
 var tap = require('tap')
-var cat = require('concat-stream')
 
 var server = require('./lib/server.js')
 var common = require('./lib/common.js')
@@ -14,10 +12,7 @@ tap.test('fetch with a 404 response', function (t) {
   server.expect('/underscore/-/underscore-1.3.3.tgz', function (req, res) {
     t.equal(req.method, 'GET', 'got expected method')
 
-    res.writeHead(200, {
-      'content-type': 'application/x-tar',
-      'content-encoding': 'gzip'
-    })
+    res.writeHead(404)
 
     createReadStream(tgz).pipe(res)
   })
@@ -27,19 +22,13 @@ tap.test('fetch with a 404 response', function (t) {
   client.fetch(
     'http://localhost:1337/underscore/-/underscore-1.3.3.tgz',
     defaulted,
-    function (er, res) {
-      t.ifError(er, 'loaded successfully')
-
-      var sink = cat(function (data) {
-        t.deepEqual(data, readFileSync(tgz))
-        t.end()
-      })
-
-      res.on('error', function (error) {
-        t.ifError(error, 'no errors on stream')
-      })
-
-      res.pipe(sink)
+    function (err, res) {
+      t.equal(
+        err.message,
+        'fetch failed with status code 404',
+        'got expected error message'
+      )
+      t.end()
     }
   )
 })

test/request.js

@@ -81,7 +81,7 @@ test('request call contract', function (t) {
 })
 
 test('run request through its paces', function (t) {
-  t.plan(29)
+  t.plan(31)
 
   server.expect('/request-defaults', function (req, res) {
     t.equal(req.method, 'GET', 'uses GET by default')
@@ -173,6 +173,13 @@ test('run request through its paces', function (t) {
     }))
   })
 
+  server.expect('GET', '/not-found-no-body', function (req, res) {
+    req.pipe(concat(function () {
+      res.statusCode = 404
+      res.end()
+    }))
+  })
+
   var defaults = {}
   client.request(
     common.registry + '/request-defaults',
@@ -260,4 +267,9 @@ test('run request through its paces', function (t) {
   client.request(common.registry + '/@scoped%2Fpackage-failing', defaults, function (er) {
     t.equals(er.message, 'payment required : @scoped/package-failing')
   })
+
+  client.request(common.registry + '/not-found-no-body', defaults, function (er) {
+    t.equals(er.message, '404 Not Found')
+    t.ok(typeof er !== 'string', "Error shouldn't be returned as string.")
+  })
 })