Return decoded_content in _unwrap_response(). #2
Conversation
|
I'm not entirely sure decoding content arbitrarily is a good idea here. Maybe it could be a constructor argument at best, but even that I'm not sure about. Because the HTTP::Tiny itself doesn't decode content and just passes it verbatim, requiring the consumer to decode correctly. ( Hence, why HTTP::Tiny::UA exists and why I presently have a feature request open for HTTP::Tiny::UA to support ->decoded_content dagolden/HTTP-Tiny-UA#3 ) So it seems what ever is consuming HTTP::Tiny::Mech and assuming it to work like HTTP::Tiny is consuming it incorrectly. Basically, I'm saying it looks like a bug in the calling code, because if you had a real HTTP::Tiny there, you'd theoretically have the same problem. |
|
Good info to have. I'll take a closer look at this to see what's going on in the calling code. |
|
Here's an example of what I'm talking about. The calling code isn't doing anything special. In this example the WWW::Mechanize object always fails for me with the decoding error I listed above. LWP::UserAgent and WWW::Mechanize::Cached are both fine. |
|
MetaCPAN::Client is naughty and quite a few calls will entirely ignore the passed ua. I'll have to look into this case and see whats happening here though. |
|
Presently having trouble even installing the dependencies as http://matrix.cpantesters.org/?dist=Cache-Cache+1.06 Somebody really aught to see if they can get some maintenance happening there. |
|
Hm, it does appear there is some different behaviour under the hood with WWW::Mechanize. WWW::Mechanize returns the header: The others do not. That is to say, the reason the others don't have the problem, is they're not getting the content encoded in the first place. |
|
And this looks like why:
So what we have happening here is this: #!/usr/bin/env perl
use strict;
use warnings;
use Devel::SimpleTrace;
use HTTP::Tiny;
use MetaCPAN::Client;
my $ua = HTTP::Tiny->new(
default_headers => {
'Accept-Encoding' => 'gzip',
},
);
my $mcpan = MetaCPAN::Client->new( ua => $ua );
$mcpan->author('XSAWYERX');I don't know what to do here, because I certainly shouldn't be sending headers myself that the user didn't ask for, and I shouldn't be decoding things in case the user is expecting to see them un-decoded. I'm kinda of the opinion that And simply arbitrarily applying the decoding is not sufficient, I must also adjust the headers in such case so that the headers no longer indicate that the content is encoded, or code will attempt to re-decode that content and then error incorrectly. Perhaps @karenetheridge can give some direction here being the maintainer/releaser/whatever for |
|
As a temporary workaround, you can avoid #!/usr/bin/env perl
use strict;
use warnings;
use Devel::SimpleTrace;
use HTTP::Tiny::Mech;
use MetaCPAN::Client;
use LWP::UserAgent;
use WWW::Mechanize;
use WWW::Mechanize::Cached;
my @ua_classes =
( 'WWW::Mechanize::Cached', 'LWP::UserAgent', 'WWW::Mechanize', );
foreach my $ua_class (@ua_classes) {
print "$ua_class\n";
my $ua = $ua_class->new( headers => { 'Accept-Encoding' => 'identity' } );
my $wrapped_ua = HTTP::Tiny::Mech->new( mechua => $ua );
my $mcpan = MetaCPAN::Client->new( ua => $wrapped_ua );
$mcpan->author('XSAWYERX');
} |
|
On Tue, Jul 22, 2014 at 10:00:35PM -0700, Kent Fredric wrote:
Heh I wish. :) I don't think I've had any contact ever with the "real" |
|
I'm just going to guess though, that WWW::Mech making this decision a while ago may mean its already unchangeable :(. My opinion is what should happen is either: a.
b.
And as such
Anything else seems to leak who's job it is to determine what encodings are an are not acceptable, and WWW::Mech decides for you, and then you get left with the pieces WWW::Mech broke. |
|
I should add to this that both WWW::Mechanize::GZip and WWW::Mechanize::Cached::GZip also don't explode with the code sample that I posted. |
|
It
Thus, it encapsulates the fact it supporting gzip and hides it from the consumer. However, it doesn't strip/fix the encoding header (https://metacpan.org/source/PEGI/WWW-Mechanize-GZip-0.12/lib/WWW/Mechanize/GZip.pm#L105) So that if something was assuming it was
|
|
Something is very wrong here and I don't know how to express it. Spot the difference: #!/usr/bin/env perl
use strict;
use warnings;
use Devel::SimpleTrace;
use HTTP::Tiny::Mech;
use MetaCPAN::Client;
use LWP::UserAgent;
use WWW::Mechanize;
use WWW::Mechanize::GZip;
use WWW::Mechanize::Cached;
use WWW::Mechanize::Cached::GZip;
use Data::Difference;
use Data::Dump qw( pp );
my @ua_classes
= ( 'WWW::Mechanize::Cached', 'LWP::UserAgent', 'WWW::Mechanize', 'WWW::Mechanize::Cached::GZip', 'WWW::Mechanize::GZip');
my $uri = 'http://api.metacpan.org/v0/author/XSAWYERX';
my $results = {};
foreach my $ua_class ( @ua_classes ) {
my $ua = $ua_class->new();
my $result = $ua->get($uri);
$results->{ $ua_class } = {
content_encoding => $result->{'_headers'}->{'content-encoding'},
content_length => $result->{'_headers'}->{'content-length'},
actual_length => length $result->content,
decoded_length => length $result->decoded_content,
};
}
print pp($results);{
"LWP::UserAgent" => {
actual_length => 826,
content_encoding => undef,
content_length => 826,
decoded_length => 826,
},
"WWW::Mechanize" => {
actual_length => 410,
content_encoding => "gzip",
content_length => undef,
decoded_length => 826,
},
"WWW::Mechanize::Cached" => {
actual_length => 826,
content_encoding => undef,
content_length => undef,
decoded_length => 826,
},
"WWW::Mechanize::Cached::GZip" => {
actual_length => 826,
content_encoding => undef,
content_length => undef,
decoded_length => 826,
},
"WWW::Mechanize::GZip" => {
actual_length => 826,
content_encoding => "gzip",
content_length => 826,
decoded_length => undef,
},
} |
|
Take special note on that last one: Yep. |
4 participants
I was using a plain WWW::Mechanize object today and I was getting the following error:
Returning decoded_content() fixes that for me.