Don't fallback between compression methods available unless we get strictly HTTP 404. #129 #125

Prior to that, some real errors could have been masked away by that fallback.

deb/remote_test.go

@@ -192,7 +192,7 @@ func (s *RemoteRepoSuite) TestFetch(c *C) {
 
 func (s *RemoteRepoSuite) TestFetchNullVerifier1(c *C) {
 	downloader := http.NewFakeDownloader()
-	downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/InRelease", errors.New("404"))
+	downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/InRelease", &http.HTTPError{Code: 404})
 	downloader.ExpectResponse("http://mirror.yandex.ru/debian/dists/squeeze/Release", exampleReleaseFile)
 	downloader.ExpectResponse("http://mirror.yandex.ru/debian/dists/squeeze/Release.gpg", "GPG")
 
@@ -252,8 +252,8 @@ func (s *RemoteRepoSuite) TestDownload(c *C) {
 	err := s.repo.Fetch(s.downloader, nil)
 	c.Assert(err, IsNil)
 
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.bz2", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.bz2", &http.HTTPError{Code: 404})
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.gz", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.gz", &http.HTTPError{Code: 404})
 	s.downloader.ExpectResponse("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages", examplePackagesFile)
 
 	err = s.repo.DownloadPackageIndexes(s.progress, s.downloader, s.collectionFactory, false)
@@ -281,11 +281,11 @@ func (s *RemoteRepoSuite) TestDownloadWithSources(c *C) {
 	err := s.repo.Fetch(s.downloader, nil)
 	c.Assert(err, IsNil)
 
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.bz2", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.bz2", &http.HTTPError{Code: 404})
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.gz", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages.gz", &http.HTTPError{Code: 404})
 	s.downloader.ExpectResponse("http://mirror.yandex.ru/debian/dists/squeeze/main/binary-i386/Packages", examplePackagesFile)
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources.bz2", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources.bz2", &http.HTTPError{Code: 404})
-	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources.gz", errors.New("HTTP 404"))
+	s.downloader.ExpectError("http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources.gz", &http.HTTPError{Code: 404})
 	s.downloader.ExpectResponse("http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources", exampleSourcesFile)
 
 	err = s.repo.DownloadPackageIndexes(s.progress, s.downloader, s.collectionFactory, false)
@@ -322,8 +322,8 @@ func (s *RemoteRepoSuite) TestDownloadWithSources(c *C) {
 func (s *RemoteRepoSuite) TestDownloadFlat(c *C) {
 	downloader := http.NewFakeDownloader()
 	downloader.ExpectResponse("http://repos.express42.com/virool/precise/Release", exampleReleaseFile)
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.bz2", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.bz2", &http.HTTPError{Code: 404})
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.gz", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.gz", &http.HTTPError{Code: 404})
 	downloader.ExpectResponse("http://repos.express42.com/virool/precise/Packages", examplePackagesFile)
 
 	err := s.flat.Fetch(downloader, nil)
@@ -352,11 +352,11 @@ func (s *RemoteRepoSuite) TestDownloadWithSourcesFlat(c *C) {
 
 	downloader := http.NewFakeDownloader()
 	downloader.ExpectResponse("http://repos.express42.com/virool/precise/Release", exampleReleaseFile)
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.bz2", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.bz2", &http.HTTPError{Code: 404})
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.gz", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Packages.gz", &http.HTTPError{Code: 404})
 	downloader.ExpectResponse("http://repos.express42.com/virool/precise/Packages", examplePackagesFile)
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Sources.bz2", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Sources.bz2", &http.HTTPError{Code: 404})
-	downloader.ExpectError("http://repos.express42.com/virool/precise/Sources.gz", errors.New("HTTP 404"))
+	downloader.ExpectError("http://repos.express42.com/virool/precise/Sources.gz", &http.HTTPError{Code: 404})
 	downloader.ExpectResponse("http://repos.express42.com/virool/precise/Sources", exampleSourcesFile)
 
 	err := s.flat.Fetch(downloader, nil)

http/download.go

@@ -16,6 +16,17 @@ import (
 	"strings"
 )
 
+// HTTPError is download error connected to HTTP code
+type HTTPError struct {
+	Code int
+	URL  string
+}
+
+// Error
+func (e *HTTPError) Error() string {
+	return fmt.Sprintf("HTTP code %d while fetching %s", e.Code, e.URL)
+}
+
 // Check interface
 var (
 	_ aptly.Downloader = (*downloaderImpl)(nil)
@@ -139,7 +150,7 @@ func (downloader *downloaderImpl) handleTask(task *downloadTask) {
 	}
 
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		task.result <- fmt.Errorf("HTTP code %d while fetching %s", resp.StatusCode, task.url)
+		task.result <- &HTTPError{Code: resp.StatusCode, URL: task.url}
 		return
 	}
 
@@ -307,13 +318,16 @@ func DownloadTryCompression(downloader aptly.Downloader, url string, expectedChe
 		}
 
 		if err != nil {
-			continue
+			if err1, ok := err.(*HTTPError); ok && err1.Code == 404 {
+				continue
+			}
+			return nil, nil, err
 		}
 
 		var uncompressed io.Reader
 		uncompressed, err = method.transformation(file)
 		if err != nil {
-			continue
+			return nil, nil, err
 		}
 
 		return uncompressed, file, err

http/download_test.go

@@ -229,7 +229,7 @@ func (s *DownloaderSuite) TestDownloadTryCompression(c *C) {
 	// bzip2 not available, but gz is
 	buf = make([]byte, 4)
 	d = NewFakeDownloader()
-	d.ExpectError("http://example.com/file.bz2", errors.New("404"))
+	d.ExpectError("http://example.com/file.bz2", &HTTPError{Code: 404})
 	d.ExpectResponse("http://example.com/file.gz", gzipData)
 	r, file, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false)
 	c.Assert(err, IsNil)
@@ -241,8 +241,8 @@ func (s *DownloaderSuite) TestDownloadTryCompression(c *C) {
 	// bzip2 & gzip not available, but raw is
 	buf = make([]byte, 4)
 	d = NewFakeDownloader()
-	d.ExpectError("http://example.com/file.bz2", errors.New("404"))
+	d.ExpectError("http://example.com/file.bz2", &HTTPError{Code: 404})
-	d.ExpectError("http://example.com/file.gz", errors.New("404"))
+	d.ExpectError("http://example.com/file.gz", &HTTPError{Code: 404})
 	d.ExpectResponse("http://example.com/file", rawData)
 	r, file, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false)
 	c.Assert(err, IsNil)
@@ -254,14 +254,10 @@ func (s *DownloaderSuite) TestDownloadTryCompression(c *C) {
 	// gzip available, but broken
 	buf = make([]byte, 4)
 	d = NewFakeDownloader()
-	d.ExpectError("http://example.com/file.bz2", errors.New("404"))
+	d.ExpectError("http://example.com/file.bz2", &HTTPError{Code: 404})
 	d.ExpectResponse("http://example.com/file.gz", "x")
-	d.ExpectResponse("http://example.com/file", "recovered")
 	r, file, err = DownloadTryCompression(d, "http://example.com/file", nil, false)
-	c.Assert(err, IsNil)
+	c.Assert(err, ErrorMatches, "unexpected EOF")
-	defer file.Close()
-	io.ReadFull(r, buf)
-	c.Assert(string(buf), Equals, "reco")
 	c.Assert(d.Empty(), Equals, true)
 }
 
@@ -271,15 +267,15 @@ func (s *DownloaderSuite) TestDownloadTryCompressionErrors(c *C) {
 	c.Assert(err, ErrorMatches, "unexpected request.*")
 
 	d = NewFakeDownloader()
-	d.ExpectError("http://example.com/file.bz2", errors.New("404"))
+	d.ExpectError("http://example.com/file.bz2", &HTTPError{Code: 404})
-	d.ExpectError("http://example.com/file.gz", errors.New("404"))
+	d.ExpectError("http://example.com/file.gz", &HTTPError{Code: 404})
 	d.ExpectError("http://example.com/file", errors.New("403"))
 	_, _, err = DownloadTryCompression(d, "http://example.com/file", nil, false)
 	c.Assert(err, ErrorMatches, "403")
 
 	d = NewFakeDownloader()
-	d.ExpectError("http://example.com/file.bz2", errors.New("404"))
+	d.ExpectError("http://example.com/file.bz2", &HTTPError{Code: 404})
-	d.ExpectError("http://example.com/file.gz", errors.New("404"))
+	d.ExpectError("http://example.com/file.gz", &HTTPError{Code: 404})
 	d.ExpectResponse("http://example.com/file", rawData)
 	_, _, err = DownloadTryCompression(d, "http://example.com/file", map[string]utils.ChecksumInfo{"file": utils.ChecksumInfo{Size: 7}}, false)
 	c.Assert(err, ErrorMatches, "checksums don't match.*")