| Subject: | Flickr::Upload doesn't check if HTTP requests were successful and gives invalid data to XML::Parser::Lite::Tree |
In Flickr::Upload::upload_request the return value of $self->request(
$req ) is only checked for definedness. Sometimes the flickr API will
fail with some error or other and close the connection without sending
any data back.
With the attached debugging (see patch) this is what flickr_upload
returns in this scenario:
"""
flickr_upload --public 0 --progress img_43??.jpg
img_4373.jpg: 12% [=============
]ETA 15:04do {
my $a = {
"\$\@" => "junk '500 Server closed connection without sending any
data back\n' before XML element\n",
is_success => "",
req => bless({
_content => sub { "???" },
_headers => bless({
"accept-encoding" => "gzip",
"content-length" => 8168700,
"content-type" => "multipart/form-data;
boundary=MSZ3KssWaEg3L2BH1ZY982eiFcgSwQcbJtjAUxjZ",
"user-agent" => "flickr_upload/1.32",
}, "HTTP::Headers"),
_method => "POST",
_uri => bless(do{\(my $o =
"http://api.flickr.com/services/upload/")}, "URI::http"),
}, "HTTP::Request"),
res => bless({
_content => "500 Server closed connection without sending any
data back\n",
_headers => bless({
"client-date" => "Wed, 14 Oct 2009 14:20:50 GMT",
"client-warning" => "Internal response",
"content-type" => "text/plain",
}, "HTTP::Headers"),
_msg => "Server closed connection without sending any data
back",
_rc => 500,
_request => 'fix',
}, "HTTP::Response"),
tree => undef,
};
$a->{res}{_request} = $a->{req};
$a;
}
"""
It should instead:
* Check if the request ->is_success
* Wrap the call to the XML parser in eval
* If either one of these fails try again say 3 times before giving up
| Subject: | flickr-upload-debugging-hack.patch |
diff --git a/Upload.pm b/Upload.pm
index 69e7e63..9608ef7 100644
--- a/Upload.pm
+++ b/Upload.pm
@@ -7,6 +7,7 @@ use LWP::UserAgent;
use HTTP::Request::Common;
use Flickr::API;
use XML::Parser::Lite::Tree;
+use Data::Dump 'dump';
our $VERSION = '1.32';
@@ -269,10 +270,39 @@ sub upload_request {
die "expecting a HTTP::Request" unless $req->isa('HTTP::Request');
my $res = $self->request( $req );
- return () unless defined $res;
-
- my $tree = XML::Parser::Lite::Tree::instance()->parse($res->decoded_content());
- return () unless defined $tree;
+ if (not $res) {
+ warn "zomg request failed";
+ print STDERR dump({
+ is_success => $res->is_success,
+ req => $req,
+ res => $res,
+ });
+ return () unless defined $res;
+ }
+
+ ## Sometimes I'll get:
+ # junk '500 read failed: Connection reset by peer
+ # ' before XML element
+ ## From XML::Parser::LiteCopy which will die
+ my $tree;
+ {
+ local ($@, $!);
+ eval {
+ $tree = XML::Parser::Lite::Tree::instance()->parse($res->decoded_content());
+ };
+
+ if ($@) {
+ print STDERR dump({
+ is_success => $res->is_success,
+ tree => $tree,
+ req => $req,
+ res => $res,
+ '$@' => $@,
+ });
+ }
+
+ return () unless defined $tree;
+ }
my $photoid = response_tag($tree, 'rsp', 'photoid');
my $ticketid = response_tag($tree, 'rsp', 'ticketid');