Retry artifact download when response stream is truncated #652
Conversation
hashtagchris
force-pushed
the
hashtagchris-artifact-flaky-response-stream
branch
from
9e42fae to
2be0cdb
Compare
| if (isGzip) { | ||
| const gunzip = zlib.createGunzip() | ||
| response.message | ||
| .on('error', error => { |
There was a problem hiding this comment.
The existing on('error', ...) callback below isn't called when gunzip fails, or response.message produces an error. pipe doesn't propagate errors across streams, so you have to monitor each. Without the new blocks the error is unhandled, and the new test times out.
I'm closing the downstream stream (hah!) on error. From https://nodejs.org/dist/latest-v14.x/docs/api/stream.html#stream_readable_pipe_destination_options:
One important caveat is that if the
Readablestream emits an error during processing, theWritabledestination is not closed automatically. If an error occurs, it will be necessary to manually close each stream in order to prevent memory leaks.
There was a problem hiding this comment.
I am a little bit unclear about when to retry here. If the download failed because the socket is opened too long (for downloading large files), would retry help?
There was a problem hiding this comment.
Good question. In short, I'm hoping retries will help. 😄
In the case of smaller files, I think there are likely transient http disconnects (timeouts?) that will benefit from retries.
For larger files (actions/download-artifact#55), is there a technical reason we'd be able to upload large files we can't download? Do we only chunk on upload? Should we add a max size check to upload-artifact if download-artifact is unlikely to succeed?
One option to add retries now, and then revisit this code if customers continue to report issues, say with larger files.
There was a problem hiding this comment.
We do chunked upload but I don't believe we do chunked download.
and then revisit this code if customers still report issues with larger files.
This is what I am afraid of, since in actions/download-artifact#55 the description says there is a 100% failure rate, I am afraid the retry will be in vain, and we will incur more minutes per build from those retries.
hashtagchris
force-pushed
the
hashtagchris-artifact-flaky-response-stream
branch
from
2be0cdb to
7ade2bb
Compare
|
#661 supersedes this PR. It includes the commits in this PR. |
Retry the artifact download when gunzip fails or the response stream is truncated.
Closes actions/download-artifact#53 and actions/download-artifact#55