Merge pull request #105 from microsoft/dev

Release 1.3.0
microsoft · Mar 12, 2024 · fc853cf · fc853cf
2 parents 71b97ef + bf8e4a1
commit fc853cf
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 8 deletions.
diff --git a/.github/workflows/conflicting-pr-label.yml b/.github/workflows/conflicting-pr-label.yml
@@ -11,6 +11,10 @@ on:
     types: [synchronize]
     branches: [ main ]
 
+permissions:
+  pull-requests: write
+  contents: read
+
 # A workflow run is made up of one or more jobs that can run sequentially or in parallel
 jobs:
   # This workflow contains a single job called "build"

diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml
@@ -7,30 +7,51 @@ on:
   pull_request:
     branches: [ main, dev ]
 
+permissions:
+  contents: read
+  pull-requests: read
+
 jobs:
   build:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        php-versions: ['7.4', '8.0', '8.1', '8.2']
+        php-versions: ['7.4', '8.1', '8.2', '8.3']
     steps:
       - name: Checkout
         uses: actions/[email protected]
-      - name: Setup PHP and Xdebug for Code Coverage report
+      - name: Setup PHP
         uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php-versions }}
-          coverage: xdebug
+          coverage: none
       - name: Install dependencies
         run: composer install
       - name: Run static analysis
         run: ./vendor/bin/phpstan
       - name: Run tests
+        run: ./vendor/bin/phpunit
+
+  code-coverage:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup PHP and Xdebug for Code Coverage report
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.0'
+          coverage: xdebug
+      - name: Install dependencies
+        run: composer install
+      - name: Run static analysis
+        run: ./vendor/bin/phpstan
+      - name: Run tests with coverage
         run: ./vendor/bin/phpunit --coverage-clover=coverage.xml
       - name: Fix code coverage paths
         run: sed -i 's@'$GITHUB_WORKSPACE'@/github/workspace/@g' coverage.xml
       - name: SonarCloud Scan
-        if: ${{ matrix.php-versions == '8.0' && !github.event.pull_request.head.repo.fork }}
+        if: ${{ !github.event.pull_request.head.repo.fork }}
         uses: SonarSource/sonarcloud-github-action@master
         env:
             GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

diff --git a/src/GuzzleRequestAdapter.php b/src/GuzzleRequestAdapter.php
@@ -637,7 +637,8 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM
             $statusCodeAsString = "$statusCode";
             if ($errorMappings === null || (!array_key_exists($statusCodeAsString, $errorMappings) &&
                     !($statusCode >= 400 && $statusCode < 500 && isset($errorMappings['4XX'])) &&
-                    !($statusCode >= 500 && $statusCode < 600 && isset($errorMappings["5XX"])))) {
+                    !($statusCode >= 500 && $statusCode < 600 && isset($errorMappings["5XX"]))) &&
+                    !isset($errorMappings['XXX'])) {
                 $span->setAttribute(self::ERROR_MAPPING_FOUND_ATTRIBUTE_NAME, false);
                 $ex = new ApiException("the server returned an unexpected status code and no error class is registered for this code $statusCode $responseBodyContents.");
                 $ex->setResponseStatusCode($response->getStatusCode());
@@ -646,7 +647,15 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM
                 throw $ex;
             }
             $span->setAttribute(self::ERROR_MAPPING_FOUND_ATTRIBUTE_NAME, true);
-            $errorClass = array_key_exists($statusCodeAsString, $errorMappings) ? $errorMappings[$statusCodeAsString] : ($errorMappings[$statusCodeAsString[0] . 'XX'] ?? null);
+
+            $errorClass = null;
+            if (array_key_exists($statusCodeAsString, $errorMappings)) { // Codes 400 - <= 599
+                $errorClass = $errorMappings[$statusCodeAsString];
+            } elseif (isset($errorMappings["$statusCodeAsString[0]XX"])) { // 5XX or 4XX
+                $errorClass = $errorMappings[$statusCodeAsString[0] . 'XX'];
+            } elseif (isset($errorMappings['XXX'])) { // The blanket case XXX.
+                $errorClass = $errorMappings['XXX'];
+            }
 
             $rootParseNode = $this->getRootParseNode($response, $errorSpan);
             if (is_null($rootParseNode)) {
@@ -658,6 +667,9 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM
                 $errorSpan->recordException($ex, ['message' => '', 'know_error' => false]);
                 throw $ex;
             }
+
+            $error = null;
+
             if ($errorClass !== null) {
                 $spanForDeserialization = $this->tracer->spanBuilder('ParseNode.GetObjectValue()')
                     ->setParent(Context::getCurrent())
@@ -668,8 +680,6 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM
                 $this->setResponseType($errorClass[0], $spanForDeserialization);
                 $spanForDeserialization->end();
 
-            } else {
-                $error = null;
             }
 
             if ($error && is_subclass_of($error, ApiException::class)) {

diff --git a/tests/GuzzleRequestAdapterTest.php b/tests/GuzzleRequestAdapterTest.php
@@ -390,6 +390,40 @@ public function testExceptionThrownOnErrorResponses(): void
         $requestAdapter->sendAsync($this->requestInformation, array(TestUser::class, 'createFromDiscriminatorValue'))->wait();
     }
 
+    public function testExceptionThrownOnErrorResponsesXXX(): void
+    {
+        $this->parseNode = $this->createStub(ParseNode::class);
+        $this->parseNodeFactory = $this->createStub(ParseNodeFactory::class);
+        $this->parseNodeFactory->method('getRootParseNode')
+            ->willReturn($this->parseNode);
+        $mockError = new MockError("Failed");
+        $this->parseNode->method('getObjectValue')
+            ->willReturn($mockError);
+        $this->expectException(MockError::class);
+        $requestAdapter = $this->mockRequestAdapter([new Response(401, ['Content-Type' => 'application/json'], '{"message" : "Failed"}')]);
+        $errorMappings = [
+            'XXX' => [MockError::class, 'createFromDiscriminatorValue']
+        ];
+        $requestAdapter->sendAsync($this->requestInformation, [TestUser::class, 'createFromDiscriminatorValue'], $errorMappings)->wait();
+    }
+
+    public function testExceptionThrownOnErrorResponses4XX(): void
+    {
+        $this->parseNode = $this->createStub(ParseNode::class);
+        $this->parseNodeFactory = $this->createStub(ParseNodeFactory::class);
+        $this->parseNodeFactory->method('getRootParseNode')
+            ->willReturn($this->parseNode);
+        $mockError = new MockError("Failed");
+        $this->parseNode->method('getObjectValue')
+            ->willReturn($mockError);
+        $this->expectException(MockError::class);
+        $requestAdapter = $this->mockRequestAdapter([new Response(400, ['Content-Type' => 'application/json'], '{"message" : "Failed 4XX"}')]);
+        $errorMappings = [
+            '4XX' => [MockError::class, 'createFromDiscriminatorValue']
+        ];
+        $requestAdapter->sendAsync($this->requestInformation, [TestUser::class, 'createFromDiscriminatorValue'], $errorMappings)->wait();
+    }
+
     public function testExceptionThrownOnErrorWithEmptyPayload(): void
     {
         $this->expectException(ApiException::class);
@@ -526,3 +560,25 @@ class TestEnum extends Enum
     const PASS = 'pass';
     const FAIL = 'fail';
 }
+
+class MockError extends ApiException implements Parsable
+{
+    public function __construct(string $message)
+    {
+        parent::__construct($message);
+    }
+    public function getFieldDeserializers(): array
+    {
+        return [];
+    }
+
+    public function serialize(SerializationWriter $writer): void
+    {
+    }
+
+    public function createFromDiscriminatorValue(ParseNode $parseNode): MockError
+    {
+        return new MockError("");
+    }
+
+}