Bug #123308 for Furl-S3: non-S3 HTTP errors are not caught

Furl::S3 passes all unsuccessful responses to Furl::S3::Error. The latter tries to extract all information from the XML body of the S3 error responses. But there are many other error conditions that don't involve S3: timeouts, connection refused, DNS failures... This patch makes Furl::S3::Error handle such failures, instead of dieing with a libxml parsing error.

diff --git c/lib/Furl/S3/Error.pm w/lib/Furl/S3/Error.pm index 16ce7d1..c4e5051 100644 --- c/lib/Furl/S3/Error.pm +++ w/lib/Furl/S3/Error.pm @@ -30,7 +30,8 @@ sub stringify { sub _parse_xml { my( $self, $xml ) = @_; - my $doc = XML::LibXML->new->parse_string( $xml ); + my $doc = eval { XML::LibXML->new->parse_string( $xml ) }; + return unless $doc; # parse failure, it's probably not an S3 error my $code = $doc->findvalue('/Error/Code'); my $message = $doc->findvalue('/Error/Message'); my $request_id = $doc->findvalue('/Error/RequestId'); diff --git c/t/03_error.t w/t/03_error.t new file mode 100644 index 0000000..097a854 --- /dev/null +++ w/t/03_error.t @@ -0,0 +1,78 @@ +#!perl +use strict; +use Test::More; +use Furl::S3::Error;; + +subtest 'can handle S3 errors' => sub { + my $err = eval { + Furl::S3::Error->new({ + 'code' => 403, + 'msg' => 'Forbidden', + 'headers' => { + 'content-type' => 'application/xml', + 'date' => 'Mon, 16 Oct 2017 11:11:17 GMT', + 'transfer-encoding' => 'chunked', + 'server' => 'AmazonS3', + }, + 'ver' => 1, + 'body' => <<'ENDBODY', +<?xml version="1.0" encoding="UTF-8"?> +<Error> + <Code>AllAccessDisabled</Code> + <Message>All access to this object has been disabled</Message> + <RequestId>myrequest</RequestId> + <HostId>myhost</HostId> +</Error> +ENDBODY + }); + }; + ok($err, 'it should live') or diag $@; + is( + $err->code, + 'AllAccessDisabled', + 'code should be extracted', + ); + is( + $err->message, + 'All access to this object has been disabled', + 'message should be extracted', + ); + is( + $err->request_id, + 'myrequest', + 'request_id should be extracted', + ); + is( + $err->host_id, + 'myhost', + 'host_id should be extracted', + ); +}; + +subtest 'it can handle HTTP errors' => sub { + my $err = eval { + Furl::S3::Error->new({ + 'code' => 500, + 'msg' => 'Internal Response: Cannot resolve host name: blahblah.localhost (port: 80), Connection timed out at lib/Furl/S3.pm line 217', + 'body' => 'Cannot resolve host name: blahblah.localhost (port: 80), Connection timed out at lib/Furl/S3.pm line 217', + 'headers' => [ + 'Content-Length' => 105, + 'X-Internal-Response' => 1, + ], + 'ver' => 0, + }); + }; + ok($err, 'it should live') or diag $@; + is( + $err->http_code, + 500, + 'http_code should be extracted', + ); + like( + $err->http_status, + qr{Cannot resolve}, + 'http_status should be extracted', + ); +}; + +done_testing;