Commit 9e31b582 authored by intrigeri's avatar intrigeri
Browse files

Upgrader: catch more download errors

As per https://metacpan.org/pod/LWP::UserAgent#request,

  Note that errors writing to the content file (for example due to permission
  denied or the filesystem being full) will be reported via the Client-Aborted
  or X-Died response headers, and not the is_success method.

We were already handling X-Died specially, but not Client-Aborted,
so there could be cases when we later treated the download as failed
(due to size or hash mismatch) but did not report about the origin
of the problem, which would make debugging of user bug reports harder.
parent 316ea48b
...@@ -76,8 +76,7 @@ Feature: download and verify a target file ...@@ -76,8 +76,7 @@ Feature: download and verify a target file
Given a HTTP server that serves "<file>" in "<webdir>" with size "<size>" and hash "<sha256>" Given a HTTP server that serves "<file>" in "<webdir>" with size "<size>" and hash "<sha256>"
When I download "<file>" (of expected size <expected_size>) from "<webdir>", and check its hash is "<sha256>" When I download "<file>" (of expected size <expected_size>) from "<webdir>", and check its hash is "<sha256>"
Then it should fail Then it should fail
And I should be told "The file '[^']*<file>' was downloaded but its size .* should be .*" And I should be told "Could not download .*[(]Client-Aborted[)]: max_size"
And the downloaded content should be not be much bigger than <expected_size>
Examples: Examples:
| file | webdir | size | expected_size | sha256 | | file | webdir | size | expected_size | sha256 |
| whatever1.file | / | 1234567 | 1 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad | | whatever1.file | / | 1234567 | 1 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad |
......
...@@ -233,19 +233,6 @@ Then qr{^the SHA-256 of the downloaded file should be "([^"]+)"$}, fun ($c) { ...@@ -233,19 +233,6 @@ Then qr{^the SHA-256 of the downloaded file should be "([^"]+)"$}, fun ($c) {
is($expected_hash, $hash_words[0]); is($expected_hash, $hash_words[0]);
}; };
Then qr{^the downloaded content should be not be much bigger than (\d+)$}, fun ($c) {
my $expected_size = $c->matches->[0];
my $not_much_bigger_base = 4096;
my $not_much_bigger_factor = 1.20;
my ($downloaded_size) =
( $c->{stash}->{scenario}->{output} =~
m{was downloaded but its size \((\d+)\) should be} );
assert(defined($downloaded_size));
ok($downloaded_size < $not_much_bigger_base + $expected_size * $not_much_bigger_factor );
};
Then qr{^the downloaded file should be world-readable$}, fun ($c) { Then qr{^the downloaded file should be world-readable$}, fun ($c) {
my $output_filename = $c->{stash}->{scenario}->{output_filename}; my $output_filename = $c->{stash}->{scenario}->{output_filename};
assert(-e $output_filename); assert(-e $output_filename);
......
...@@ -120,11 +120,13 @@ method run () { ...@@ -120,11 +120,13 @@ method run () {
$self->uri, $temp_filename, $self->uri, $temp_filename,
)); ));
my $died_header = $res->header('X-Died'); for my $lwp_failure_header (qw{Client-Aborted X-Died}) {
! defined $died_header or clean_fatal($self, $temp_filename, sprintf( my $header = $res->header($lwp_failure_header);
"Could not download '%s' to '%s', callback died:\n%s", ! defined $header or clean_fatal($self, $temp_filename, sprintf(
$self->uri, $temp_filename, $died_header, "Could not download '%s' to '%s' (%s): %s",
$self->uri, $temp_filename, $lwp_failure_header, $header,
)); ));
}
$res->is_success or clean_fatal($self, $temp_filename, sprintf( $res->is_success or clean_fatal($self, $temp_filename, sprintf(
"Could not download '%s' to '%s', request failed:\n%s\n", "Could not download '%s' to '%s', request failed:\n%s\n",
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment