8000 Fix path when possible when an exception occurs by lyrixx · Pull Request #4643 · twigphp/Twig · GitHub
[go: up one dir, main page]

Skip to content

Fix path when possible when an exception occurs #4643

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

Closed
wants to merge 1 commit into from

Conversation

lyrixx
Copy link
Contributor
@lyrixx lyrixx commented May 28, 2025

I found an error in the integration between Twig and Symfony
If a template throw an error, Symfony will enhance the message.
If a path is found in the message, it'll turn it to a link

image

https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/ErrorHandler/ErrorRenderer/HtmlErrorRenderer.php#L304-L307
https://github.com/symfony/symfony/blob/7.4/src/Symfony/Component/ErrorHandler/Resources/views/exception.html.php#L18

However, in twig, we don't put an absolute path, but a relative one.
So it does not work. This PR puts an absolute link if possible

Caution

I did not updated the test yet, I'm waiting for feedback

@@ -122,8 +122,12 @@ private function updateRepr(): void
if ($punctuation = '.' === $last || '?' === $last ? $last : '') {
$this->message = substr($this->message, 0, -1);
}
if ($this->source && $this->source->getName()) {
$this->message .= \sprintf(' in "%s"', $this->source->getName());
if ($this->source) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks wrong to me. Here, we want to use the name of the template, not the path. The path/line information is part of the regular Exception properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we need to do that to make Symfony works. I don't know how we can do better.

@stof
Copy link
Member
stof commented May 28, 2025

As discussed in the Symfony Slack channel, the HtmlErrorRenderer of Symfony must be fixed to stop assuming that anything looking like in "..." in an exception message corresponds to ... being a valid absolute path.

However, in twig, we don't put an absolute path, but a relative one.

We actually don't put a path at all (even relative). We put a Twig template name.

@fabpot fabpot closed this May 29, 2025
fabpot added a commit to symfony/symfony that referenced this pull request May 29, 2025
…t exist (lyrixx)

This PR was merged into the 6.4 branch.

Discussion
----------

[ErrorHandler] Do not transform file to link if it does not exist

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

see twigphp/Twig#4643

Commits
-------

2dfac6b [ErrorHandler] Do not transform file to link if it does not exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0