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

More complete Display implementation for Error #915

Open
Thomasdezeeuw opened this issue May 17, 2024 · 5 comments
Open

More complete Display implementation for Error #915

Thomasdezeeuw opened this issue May 17, 2024 · 5 comments

Comments

@Thomasdezeeuw
Copy link
Contributor

We've been hitting some template errors, usually nothing major, but the error message were never very useful. Usually they are just Failed to render '$FILE'", that's it. I just realised that Error actually contains more information, for example Variable `$VAR` not found in context while rendering '$FILE'. However we've missed those complete because we print out error using {}, i.e. the standard way of printing error.

Could Error::source also be included in the Display implementation of Error? This would make debug a whole lot easier for us.

@Keats
Copy link
Owner

Keats commented May 17, 2024

I don't remember why it's done that way tbh

@Thomasdezeeuw
Copy link
Contributor Author

Would you accept a pr to include the source information in the Display implementation?

@Keats
Copy link
Owner

Keats commented May 20, 2024

I'm not sure, would that be considered a breaking change?

@Thomasdezeeuw
Copy link
Contributor Author

I'm not sure, would that be considered a breaking change?

It's hard to say. I think the worse case is if people already worked around this issue and printed the source error themselves, leading to the source error being printed twice. Which, while not ideal, isn't the end of the world either?

@Keats
Copy link
Owner

Keats commented May 22, 2024

I think i'd consider a breaking change but mostly for the end user. If it's included in Display, we will see the source errors twice for apps that did a workaround as you say.
I'd prefer keeping that change for the next breaking version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants