Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#39415 - Fixed an issue where the ExceptionHandler did not display the correct file and line number of the thrown exception. #39453

Open
wants to merge 14 commits into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions lib/internal/Magento/Framework/App/ExceptionHandler.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
* Copyright 2024 Adobe
* All Rights Reserved.
*/

declare(strict_types=1);

namespace Magento\Framework\App;
Expand Down Expand Up @@ -107,6 +108,27 @@ private function handleDeveloperMode(
return false;
}

/**
* Retrieves the location of an exception as a formatted string.
*
* This method extracts the file path and line number from the given exception
* and formats them into a string representing the exception's location.
* The format of the returned string is determined by the `normalizeCodeLocation` method.
*
* Example Output:
* "app/code/Vendor/Module/SomeClass.php:123"
*
* @param \Exception $exception The exception object from which to extract the file and line information.
* @return string A formatted string representing the exception's file path and line number.
*/
private function getExceptionLocation(\Exception $exception): string
{
return Debug::normalizeCodeLocation([
'file' => $exception->getFile(),
'line' => $exception->getLine()
]);
}

/**
* Build content based on an exception
*
Expand All @@ -126,19 +148,21 @@ private function buildContentFromException(\Exception $exception): string

foreach ($exceptions as $index => $exception) {
$buffer .= sprintf(
"Exception #%d (%s): %s\n",
"Exception #%d (%s): \"%s\" thrown at [%s]\n",
$index,
get_class($exception),
$exception->getMessage()
$exception->getMessage(),
$this->getExceptionLocation($exception)
);
}

foreach ($exceptions as $index => $exception) {
$buffer .= sprintf(
"\nException #%d (%s): %s\n%s\n",
"\nException #%d (%s): \"%s\" thrown at [%s]\n%s\n",
$index,
get_class($exception),
$exception->getMessage(),
$this->getExceptionLocation($exception),
Debug::trace(
$exception->getTrace(),
true,
Expand Down
74 changes: 59 additions & 15 deletions lib/internal/Magento/Framework/Debug.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
* Copyright 2024 Adobe
* All Rights Reserved.
*/

namespace Magento\Framework;

/**
Expand Down Expand Up @@ -39,6 +40,52 @@ public static function getRootPath()
return self::$_filePath;
}

/**
* Formats the code location by generating a relative file path and line number.
*
* This method processes an input array that contains file path and line number information.
* It removes the root path from the file path (if present) to create a relative path
* and formats the result as a string in the format "relative/path/to/file.php:line".
*
* Example Input:
* [
* 'file' => '/var/www/magento2/app/code/Magento/Test/Block/Test.php',
* 'line' => 1
* ]
*
* Example Output:
* "app/code/Magento/Test/Block/Test.php:1"
*
* @param array|null $data An associative array containing:
* - 'file': The absolute file path (string).
* - 'line': The line number (int, optional).
* @return string A formatted string representing the relative file path and line number,
* or an empty string if the 'file' key is not present.
*/
public static function normalizeCodeLocation(?array $data): string
{
$fileName = '';

// Check if 'file' exists in the data array
if (isset($data['file'])) {
// Determine the position of the root path in the file path
$pos = \strpos($data['file'], self::getRootPath());

// If Magento root path is part of the file path, trim it to create a relative path
if (false !== $pos) {
$data['file'] = \substr(
$data['file'],
\strlen(self::getRootPath()) + 1
);
}

// Format the file path and line number into the desired string format
$fileName = \sprintf('%s:%d', $data['file'], $data['line'] ?? 0);
}

return $fileName;
}

/**
* Prints or returns a backtrace
*
Expand All @@ -56,10 +103,10 @@ public static function backtrace($return = false, $html = true, $withArgs = true
/**
* Prints or return a trace
*
* @param array $trace trace array
* @param bool $return return or print
* @param bool $html output in HTML format
* @param bool $withArgs add short arguments of methods
* @param array $trace trace array
* @param bool $return return or print
* @param bool $html output in HTML format
* @param bool $withArgs add short arguments of methods
* @return string|bool
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
Expand All @@ -85,6 +132,9 @@ public static function trace(array $trace, $return = false, $html = true, $withA
}
}

// Fix static test: 'Variable $methodName might not be defined.'
$methodName = '';

// prepare method's name
if (isset($data['class']) && isset($data['function'])) {
if (isset($data['object']) && get_class($data['object']) != $data['class']) {
Expand All @@ -107,15 +157,7 @@ public static function trace(array $trace, $return = false, $html = true, $withA
$methodName = sprintf('%s(%s)', $data['function'], join(', ', $args));
}

if (isset($data['file'])) {
$pos = strpos($data['file'], self::getRootPath());
if ($pos !== false) {
$data['file'] = substr($data['file'], strlen(self::getRootPath()) + 1);
}
$fileName = sprintf('%s:%d', $data['file'], $data['line']);
} else {
$fileName = false;
}
$fileName = self::normalizeCodeLocation($data);

if ($fileName) {
$out .= sprintf('#%d %s called at [%s]', $i, $methodName, $fileName);
Expand All @@ -133,6 +175,8 @@ public static function trace(array $trace, $return = false, $html = true, $withA
if ($return) {
return $out;
} else {
// Fix static test: 'Use of echo language construct is discouraged.'
// phpcs:ignore Magento2.Security.LanguageConstruct.DirectOutput
echo $out;
return true;
}
Expand Down