Check http status code on feed downloads for a more precise error message

[EricShapiro]

Summary

Sparkle 2.0.0 is displaying "An error occurred while parsing the update feed" when the problem is actually a permission error from the web server. Checking the http statusCode would allow for a better error message.

Details:
Our feed is protected by CloudFront. If the user's access token expires, CloudFront returns a 503 error along with an HTML page explaining the issue. Sparkle's NSURLSessionDownloadTask delegate doesn't check the http status code on the download and attempts to parse the HTML error page from CloudFront. This results in the slightly erroneous but confusing error message above.

Possible Fix

I believe this can be fixed in SPUDownloader.m: didFinishDownloadingToURL by checking the statusCode and using a new error message if it indicates an error. A (Swift - sorry) example on checking the statusCode is shown below:

    func urlSession(_ session: URLSession, 
      downloadTask: URLSessionDownloadTask, 
      didFinishDownloadingTo location: URL) {

      if let response = downloadTask.response as? HTTPURLResponse {
        if response.statusCode < 200 || response.statusCode >= 400 {
          error = "Status code \(response.statusCode)"
          return
        }
      }
      // download was successful
    }

An improved message might be something as simple as "A network error occurred while downloading the update feed (503)."

Version

2.0.0

[zorgiepoo]

We display a generic error (which is localizable and has translations) to the user and log out the underlying NSError from NSURLSession (from - (void)URLSession:(NSURLSession *)__unused session task:(NSURLSessionTask *)__unused task didCompleteWithError:(NSError *)error). To users the error message suggested here is not much more helpful. To developers I believe the underlying error is logged out in more detail. Likely 1.x would show this same error message too.

[EricShapiro]

The error in this case is nil within didCompleteWithError. I guess that's just how URLSessionDownloadTask works - it's not http specific so doesn't check http error codes.

Certain generic connection errors, like server not found, do appear in that routine, but not http status codes.

[zorgiepoo]

Ah right, this error is happening later on when trying to parse the downloaded contents.. Then yeah it would be an improvement to check the http status code and maybe re-use the An error occurred while downloading the update. Please try again later. string, and maybe append the status code if it's not an issue grammar-wise with all languages.

Feel free to make a PR on this.

[zorgiepoo]

Err that string I copied/pasted doesn't make sense :). We can have a new localizable string for this then.