Переглянути джерело

Fix #142: Die in the event of HTTP 401/403

George S. Baugh 7 роки тому
батько
коміт
71c341cd1b
4 змінених файлів з 64 додано та 5 видалено
  1. 1 0
      Changes
  2. 9 4
      lib/TestRail/API.pm
  3. 16 1
      t/TestRail-API-mockOnly.t
  4. 38 0
      t/lib/Test/LWP/UserAgent/TestRailMock.pm

+ 1 - 0
Changes

@@ -2,6 +2,7 @@ Revision history for Perl module TestRail::API
 
 0.042 2018-04-30 TEODESIAN
     - Fix uninitialized value warning when no plan is emitted
+    - Die on HTTP 401/403, we likely will never recover from this
 
 0.041 2017-06-06 TEODESIAN
     - Fix MCE usage issue with confusion based on array -> hash inputs in TestRail::Utils::Find

+ 9 - 4
lib/TestRail/API.pm

@@ -17,7 +17,7 @@ It is by no means exhaustively implementing every TestRail API function.
 
 =head1 IMPORTANT
 
-All the methods aside from the constructor should not die, but return a false value upon failure.
+All the methods aside from the constructor should not die, but return a false value upon failure (see exceptions below).
 When the server is not responsive, expect a -500 response, and retry accordingly.
 I recommend using the excellent L<Attempt> module for this purpose.
 
@@ -32,6 +32,8 @@ Also, all *ByName methods are vulnerable to duplicate naming issues.  Try not to
 
 To do so will result in the first of said item found being returned rather than an array of possibilities to choose from.
 
+There are two exceptions to this, in the case of 401 and 403 responses, as these failing generally mean your program has no chance of success anyways.
+
 =cut
 
 use 5.010;
@@ -204,11 +206,14 @@ sub _doRequest {
 
     return $response if !defined($response); #worst case
     if ($response->code == 403) {
-        cluck "ERROR: Access Denied.";
-        return -403;
+        confess "ERROR 403: Access Denied: ".$response->content;
+    }
+    if ($response->code == 401) {
+        confess "ERROR 401: Authentication failed: ".$response->content;
     }
+
     if ($response->code != 200) {
-        cluck "ERROR: Arguments Bad: ".$response->content;
+        cluck "ERROR: Arguments Bad? (got code ".$response->code."): ".$response->content;
         return -int($response->code);
     }
 

+ 16 - 1
t/TestRail-API-mockOnly.t

@@ -6,11 +6,12 @@ use lib "$FindBin::Bin/lib";
 
 #Test things we can only mock, because the API doesn't support them.
 
-use Test::More 'tests' => 14;
+use Test::More 'tests' => 16;
 use TestRail::API;
 use Test::LWP::UserAgent::TestRailMock;
 use Scalar::Util qw{reftype};
 use Capture::Tiny qw{capture};
+use Test::Fatal;
 
 my $browser = $Test::LWP::UserAgent::TestRailMock::mockObject;
 my $tr = TestRail::API->new('http://hokum.bogus','fake','fake',undef,1);
@@ -51,3 +52,17 @@ is($res,-404,"Can't close plan that doesn't exist");
 # Test case type method
 my $ct = $tr->getCaseTypeByName("Automated");
 is($ct->{'id'},1,"Can get case type by name");
+
+#Test #142
+$tr = TestRail::API->new('http://locked.out','fake','fake',undef,1);
+$tr->{'browser'} = $browser;
+$tr->{'debug'} = 0;
+
+like( exception { $tr->getUsers() } , qr/stay out you red menace/i, "API dies on bad auth");
+
+$tr = TestRail::API->new('http://locked.out/worse','fake','fake',undef,1);
+$tr->{'browser'} = $browser;
+$tr->{'debug'} = 0;
+
+like( exception { $tr->getUsers() } , qr/could not find pants/i, "API dies on no auth or auth backend failure");
+

+ 38 - 0
t/lib/Test/LWP/UserAgent/TestRailMock.pm

@@ -38,6 +38,44 @@ my ($VAR1,$VAR2,$VAR3,$VAR4,$VAR5);
 
 {
 
+$VAR1 = 'http://locked.out/index.php?/api/v2/get_users';
+$VAR2 = 403;
+$VAR3 = 'Stay out you red menace';
+$VAR4 = bless( {
+                 'client-warning' => 'Internal response',
+                 'client-date' => 'Tue, 23 Dec 2014 20:02:08 GMT',
+                 'content-type' => 'text/plain',
+                 '::std_case' => {
+                                   'client-warning' => 'Client-Warning',
+                                   'client-date' => 'Client-Date'
+                                 }
+               }, 'HTTP::Headers' );
+$VAR5 = 'Stay out you red menace';
+$mockObject->map_response(qr/\Q$VAR1\E/,HTTP::Response->new($VAR2, $VAR3, $VAR4, $VAR5));
+
+}
+
+{
+
+$VAR1 = 'http://locked.out/worse/index.php?/api/v2/get_users';
+$VAR2 = 401;
+$VAR3 = 'Could not find pants with both hands';
+$VAR4 = bless( {
+                 'client-warning' => 'Internal response',
+                 'client-date' => 'Tue, 23 Dec 2014 20:02:08 GMT',
+                 'content-type' => 'text/plain',
+                 '::std_case' => {
+                                   'client-warning' => 'Client-Warning',
+                                   'client-date' => 'Client-Date'
+                                 }
+               }, 'HTTP::Headers' );
+$VAR5 = 'Could not find pants with both hands';
+$mockObject->map_response(qr/\Q$VAR1\E/,HTTP::Response->new($VAR2, $VAR3, $VAR4, $VAR5));
+
+}
+
+{
+
 $VAR1 = 'http://hokum.bogus/index.php?/api/v2/get_users';
 $VAR2 = 500;
 $VAR3 = 'Can\'t connect to hokum.bogus:80 (Bad hostname)';