Merge pull request #1065 from atom/wl-ensure-json

Make sure body was successfully parsed as JSON before filtering

Author: Winston Liu

lib/atom-io-client.coffee

@@ -69,19 +69,25 @@ class AtomIoClient
     options = {
       url: "#{@baseURL}#{path}"
       headers: {'User-Agent': navigator.userAgent}
-      json: true
       gzip: true
     }
 
     request options, (err, res, body) =>
       return callback(err) if err
 
-      delete body.versions
-      cached =
-        data: body
-        createdOn: Date.now()
-      localStorage.setItem(@cacheKeyForPath(path), JSON.stringify(cached))
-      callback(err, cached.data)
+      try
+        # NOTE: request's json option does not populate err if parsing fails,
+        # so we do it manually
+        body = JSON.parse(body)
+        delete body.versions
+
+        cached =
+          data: body
+          createdOn: Date.now()
+        localStorage.setItem(@cacheKeyForPath(path), JSON.stringify(cached))
+        callback(err, cached.data)
+      catch error
+        callback(error)
 
   cacheKeyForPath: (path) ->
     "settings-view:#{path}"
@@ -199,7 +205,6 @@ class AtomIoClient
       url: "#{@baseURL}packages/search"
       headers: {'User-Agent': navigator.userAgent}
       qs: qs
-      json: true
       gzip: true
     }
 
@@ -210,9 +215,17 @@ class AtomIoClient
           error.stderr = err.message
           reject error
         else
-          resolve(
-            body.filter (pkg) -> pkg.releases?.latest?
-                .map ({readme, metadata, downloads, stargazers_count}) ->
-                  Object.assign metadata, {readme, downloads, stargazers_count}
-                .sort (a, b) -> b.downloads - a.downloads
-          )
+          try
+            # NOTE: request's json option does not populate err if parsing fails,
+            # so we do it manually
+            body = JSON.parse(body)
+            resolve(
+              body.filter (pkg) -> pkg.releases?.latest?
+                  .map ({readme, metadata, downloads, stargazers_count}) ->
+                    Object.assign metadata, {readme, downloads, stargazers_count}
+                  .sort (a, b) -> b.downloads - a.downloads
+            )
+          catch e
+            error = new Error("Searching for \u201C#{query}\u201D failed.")
+            error.stderr = e.message + '\n' + body
+            reject error

spec/atom-io-client-spec.coffee

@@ -11,15 +11,29 @@ describe "AtomIoClient", ->
     expect(@client.fetchAndCacheAvatar).not.toHaveBeenCalled()
     @client.avatar 'test-user', ->
 
-  it "fetches api json from cache if the network is unavailable", ->
-    spyOn(@client, 'online').andReturn(false)
-    spyOn(@client, 'fetchFromCache').andCallFake (path, opts, cb) ->
-      cb(null, {})
-    spyOn(@client, 'request')
-    @client.package 'test-package', ->
-
-    expect(@client.fetchFromCache).toHaveBeenCalled()
-    expect(@client.request).not.toHaveBeenCalled()
+  describe "request", ->
+    it "fetches api json from cache if the network is unavailable", ->
+      spyOn(@client, 'online').andReturn(false)
+      spyOn(@client, 'fetchFromCache').andCallFake (path, opts, cb) ->
+        cb(null, {})
+      spyOn(@client, 'request')
+      @client.package 'test-package', ->
+
+      expect(@client.fetchFromCache).toHaveBeenCalled()
+      expect(@client.request).not.toHaveBeenCalled()
+
+    it "returns an error if the API response is not JSON", ->
+      jsonParse = JSON.parse
+
+      waitsFor (done) ->
+        spyOn(JSON, 'parse').andThrow()
+        @client.request 'path', (error, data) ->
+          expect(error).not.toBeNull()
+          done()
+
+      runs ->
+        # Tests will throw without this as cleanup requires JSON.parse to work
+        JSON.parse = jsonParse
 
   it "handles glob errors", ->
     spyOn(@client, 'avatarGlob').andReturn "#{__dirname}/**"