[api] Cache HTTP 4xx server responses (except 403)

Assume that 4xx responses will be the same given the same request, so
these responses should be subject to caching too.

Avoids caching 403 because that is the status returned when GitHub API
limit is hit.

Continue not caching 5xx responses because they indicate server errors,
so it's possible that a 2nd request of identical parameters succeeds.

commands/api.go

@@ -201,7 +201,6 @@ func apiCommand(cmd *Command, args *Args) {
 	gh := github.NewClient(host)
 	response, err := gh.GenericAPIRequest(method, path, body, headers, cacheTTL)
 	utils.Check(err)
-	defer response.Body.Close()
 
 	args.NoForward()
 
@@ -226,6 +225,7 @@ func apiCommand(cmd *Command, args *Args) {
 	} else {
 		io.Copy(out, response.Body)
 	}
+	response.Body.Close()
 
 	if !success {
 		os.Exit(22)

features/api.feature

@@ -309,13 +309,32 @@ Feature: hub api
       .count	2\n
       """
 
-  Scenario: Avoid caching unsucessful response
+  Scenario: Cache client error response
     Given the GitHub API server:
       """
       count = 0
       get('/count') {
         count += 1
-        status 400 if count == 1
+        status 404 if count == 1
+        json :count => count
+      }
+      """
+    When I run `hub api -t count --cache 5`
+    And I run `hub api -t count --cache 5`
+    Then the output should contain exactly:
+      """
+      .count	1
+      .count	1\n
+      """
+    And the exit status should be 22
+
+  Scenario: Avoid caching server error response
+    Given the GitHub API server:
+      """
+      count = 0
+      get('/count') {
+        count += 1
+        status 500 if count == 1
         json :count => count
       }
       """

github/http.go

@@ -327,7 +327,7 @@ func (c *simpleClient) cacheRead(key string, req *http.Request) (res *http.Respo
 }
 
 func (c *simpleClient) cacheWrite(key string, res *http.Response) {
-	if c.CacheTTL > 0 && canCache(res.Request) && res.StatusCode < 300 && res.Body != nil {
+	if c.CacheTTL > 0 && canCache(res.Request) && res.StatusCode < 500 && res.StatusCode != 403 {
 		bodyCopy := &bytes.Buffer{}
 		bodyReplacement := readCloserCallback{
 			Reader: io.TeeReader(res.Body, bodyCopy),