Retry artifact download when response stream is truncated (continued)

[yacaovsnc]

Continuation of #652

Closes actions/download-artifact#53 and actions/download-artifact#55

We retry when we receive some non success http status code such as 409, 503, etc. However even if we received a 200 success status code but the response stream were interrupted during download, we should also retry.

When we stream down the file, we could stream with gzip compression, and if we do, we rely on gzip processing to detect the stream is corrupted. If the stream is not gzip compressed, we will verify the downloaded file is of the same size specified in Content-Length header.

[yacaovsnc]

I tested all the positive cases, not sure how to test the negative case since my download was working fine. I tested with 1GB files.

Tested:

1. Download incompressible file, so no gzip.
2. Download highly compressible file with gzip stream
3. Skipping file size check by setting the env var.

Headers:

Total number of files that will be downloaded: 2
##### Begin Diagnostic HTTP information #####
Status Code: 200
Status Message: OK
Header Information: {
  "cache-control": "no-store,no-cache",
  "pragma": "no-cache",
  "content-length": "1043656",
  "content-type": "application/octet-stream",
  "content-encoding": "gzip",
  "strict-transport-security": "max-age=2592000",
  "x-tfs-processid": "8e7af2a5-0c3f-45e5-9baa-1caea8857637",
  "activityid": "55af6bc4-e899-40ce-87cd-a8c28b7be3bc",
  "x-tfs-session": "55af6bc4-e899-40ce-87cd-a8c28b7be3bc",
  "x-vss-e2eid": "55af6bc4-e899-40ce-87cd-a8c28b7be3bc",
  "x-vss-senderdeploymentid": "193695a0-0dcd-ade4-f810-b10ad24a9829",
  "x-frame-options": "SAMEORIGIN",
  "content-disposition": "attachment; filename=zero.bin; filename*=UTF-8''zero.bin",
  "x-msedge-ref": "Ref A: 4276B404BFFE4DE7A3F445E368977664 Ref B: BL2EDGE0912 Ref C: 2020-12-03T17:37:15Z",
  "date": "Thu, 03 Dec 2020 17:37:14 GMT"
}
###### End Diagnostic HTTP information ######
##### Begin Diagnostic HTTP information #####
Status Code: 200
Status Message: OK
Header Information: {
  "cache-control": "no-store,no-cache",
  "pragma": "no-cache",
  "content-length": "1073741824",
  "content-type": "application/octet-stream",
  "strict-transport-security": "max-age=2592000",
  "x-tfs-processid": "27718d9d-1c29-459a-b6f7-1de5dad76990",
  "activityid": "4708872f-155c-4da7-af4b-d6e153e56a0c",
  "x-tfs-session": "4708872f-155c-4da7-af4b-d6e153e56a0c",
  "x-vss-e2eid": "4708872f-155c-4da7-af4b-d6e153e56a0c",
  "x-vss-senderdeploymentid": "193695a0-0dcd-ade4-f810-b10ad24a9829",
  "x-frame-options": "SAMEORIGIN",
  "content-disposition": "attachment; filename=random.bin; filename*=UTF-8''random.bin",
  "x-msedge-ref": "Ref A: 0363D65B20FD441E8978A56FD94F8DA6 Ref B: ASHEDGE1209 Ref C: 2020-12-03T17:37:15Z",
  "date": "Thu, 03 Dec 2020 17:37:14 GMT"
}

[konradpabjan]

Changes look really good and the new tests are also 👍

One concern is regarding some failing tests, there might be some issues on Windows:

[yacaovsnc]

One concern is regarding some failing tests, there might be some issues on Windows:

Seems like the stream is not closed on Windows so we couldn't unlink the file. 🤦

[yacaovsnc]

@konradpabjan @hashtagchris do you mind to take another look? I fixed the unit test. Thanks!

[konradpabjan]

Looks good! 🚀

[hashtagchris]

Looks good - thanks for fixing this!

[hashtagchris]

FYI, I was a little worried that calling gunzip.close() would trigger on error below and cause two core.errors to get logged. But I tried adding the following to the mock, and saw only this error block get hit:

          case 1:
            this.emit("error", new Error("injecting read error"))
            break