Fix zf1 exception handling

DataDog · Mar 10, 2023 · 477f31b · 477f31b
1 parent 033de3c
commit 477f31b
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 34 deletions.
diff --git a/src/Integrations/Integrations/ZendFramework/ZendFrameworkIntegration.php b/src/Integrations/Integrations/ZendFramework/ZendFrameworkIntegration.php
@@ -53,17 +53,15 @@ public function init()
         // name 'zendframework', we are instead using the 'zf1' prefix.
         $appName = \ddtrace_config_app_name('zf1');
 
-        \DDTrace\trace_method(
+        \DDTrace\hook_method(
             'Zend_Controller_Plugin_Broker',
             'preDispatch',
-            function (SpanData $spanData, $args) use ($integration, $appName) {
+            function ($broker, $scope, $args) use ($integration, $appName) {
                 $rootSpan = \DDTrace\root_span();
                 if (null === $rootSpan) {
-                    return false;
+                    return;
                 }
 
-                // We are enclosing this into a try-catch because we always have to return false,
-                // even at the cost of not setting specific metadata.
                 try {
                     /** @var Zend_Controller_Request_Abstract $request */
                     list($request) = $args;
@@ -90,25 +88,18 @@ function (SpanData $spanData, $args) use ($integration, $appName) {
                     }
                 } catch (\Exception $e) {
                 }
-
-                return false;
             }
         );
 
-        \DDTrace\trace_method('Zend_Controller_Plugin_Broker', 'postDispatch', function () {
+        \DDTrace\hook_method('Zend_Controller_Plugin_Broker', 'postDispatch', null, function ($broker) {
             $rootSpan = \DDTrace\root_span();
             if (null === $rootSpan) {
-                return false;
+                return;
             }
 
-            // We are enclosing this into a try-catch because we always have to return false,
-            // even at the cost of not setting specific metadata.
-            try {
-                $rootSpan->meta[Tag::HTTP_STATUS_CODE] = $this->getResponse()->getHttpResponseCode();
-            } catch (\Exception $e) {
+            if ($exceptions = $broker->getResponse()->getException()) {
+                $rootSpan->exception = reset($exceptions);
             }
-
-            return false;
         });
 
         return Integration::LOADED;

diff --git a/tests/Frameworks/ZendFramework/Version_1_21/application/controllers/ErrorController.php b/tests/Frameworks/ZendFramework/Version_1_21/application/controllers/ErrorController.php
@@ -10,12 +10,12 @@ public function indexAction()
     public function errorAction()
     {
         $errors = $this->_getParam('error_handler');
-        
+
         if (!$errors || !$errors instanceof ArrayObject) {
             $this->view->message = 'You have reached the error page';
             return;
         }
-        
+
         switch ($errors->type) {
             case Zend_Controller_Plugin_ErrorHandler::EXCEPTION_NO_ROUTE:
             case Zend_Controller_Plugin_ErrorHandler::EXCEPTION_NO_CONTROLLER:
@@ -32,18 +32,18 @@ public function errorAction()
                 $this->view->message = 'Application error';
                 break;
         }
-        
+
         // Log exception, if logger available
         if ($log = $this->getLog()) {
             $log->log($this->view->message, $priority, $errors->exception);
             $log->log('Request Parameters', $priority, $errors->request->getParams());
         }
-        
+
         // conditionally display exceptions
         if ($this->getInvokeArg('displayExceptions') == true) {
             $this->view->exception = $errors->exception;
         }
-        
+
         $this->view->request   = $errors->request;
     }
 

diff --git a/tests/Frameworks/ZendFramework/Version_1_21/public/index.php b/tests/Frameworks/ZendFramework/Version_1_21/public/index.php
@@ -23,4 +23,4 @@
     APPLICATION_PATH . '/configs/application.ini'
 );
 $application->bootstrap()
-            ->run();
+            ->run();
diff --git a/tests/Integrations/ZendFramework/V1/CommonScenariosTest.php b/tests/Integrations/ZendFramework/V1/CommonScenariosTest.php
@@ -69,14 +69,10 @@ public function provideSpecs()
                         'http.status_code' => '500',
                         Tag::SPAN_KIND => "server",
                         Tag::COMPONENT => "zendframework",
-                    ]),
+                    ])->setError('Exception', 'Controller error', true),
             ],
         ];
-        if (\getenv('DD_TRACE_TEST_SAPI') == 'apache2handler') { // i.e. with opcache
-            $specs['A GET request with an exception'][0]->setError('Exception', 'Controller error', true);
-        } else {
-            $specs['A GET request with an exception'][0]->setError();
-        }
+
         return $this->buildDataProvider($specs);
     }
 }
diff --git a/tests/Integrations/ZendFramework/V1_21/CommonScenariosTest.php b/tests/Integrations/ZendFramework/V1_21/CommonScenariosTest.php
@@ -69,14 +69,11 @@ public function provideSpecs()
                         'http.status_code' => '500',
                         Tag::SPAN_KIND => "server",
                         Tag::COMPONENT => "zendframework",
-                    ]),
+                    ])
+                    ->setError('Exception', 'Controller error', true)
             ],
         ];
-        if (\getenv('DD_TRACE_TEST_SAPI') == 'apache2handler') { // i.e. with opcache
-            $specs['A GET request with an exception'][0]->setError('Exception', 'Controller error', true);
-        } else {
-            $specs['A GET request with an exception'][0]->setError();
-        }
+
         return $this->buildDataProvider($specs);
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -23,4 +23,4 @@ @@
         APPLICATION_PATH . '/configs/application.ini'
     );
     $application->bootstrap()
-                ->run();
+                ->run();