Skip to content

Place wrapper around comment as diff to catch panics #15085

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

Merged
merged 5 commits into from
Mar 21, 2021

Conversation

zeripath
Copy link
Contributor

There are a few recurrent issues with comment as diff reporting panics that are resistant to fixing due to the fact that the panic occurs in the template render and is swallowed by the template renderer.

This PR just adds some logging to force the panic to properly logged and re-propagates back up to the template renderer so we can actually detect what the issue is.

Signed-off-by: Andrew Thornton [email protected]

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 21, 2021
@zeripath zeripath changed the title Place wrapper around comment as diff to prevent panics Place wrapper around comment as diff to catch panics Mar 21, 2021
@6543 6543 added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Mar 21, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 21, 2021
6543 pushed a commit that referenced this pull request Mar 21, 2021
* Place wrapper around comment as diff to prevent panics

* propagate the panic up

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath
Copy link
Contributor Author

I've stopped the propagation of the panic up as we probably don't want it in 1.14 and 1.15

@zeripath
Copy link
Contributor Author

make lg-tm work

@zeripath zeripath merged commit 46782d5 into go-gitea:master Mar 21, 2021
@zeripath zeripath deleted the comment-as-diff-panic-wrapper branch March 21, 2021 17:00
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 21, 2021
Backport go-gitea#15085

There are a few recurrent issues with comment as diff reporting panics that are resistant to fixing due to the fact that the panic occurs in the template render and is swallowed by the template renderer.

This PR just adds some logging to force the panic to properly logged and re-propagates back up to the template renderer so we can actually detect what the issue is.

Signed-off-by: Andrew Thornton [email protected]
6543 pushed a commit that referenced this pull request Mar 21, 2021
Backport #15085

There are a few recurrent issues with comment as diff reporting panics that are resistant to fixing due to the fact that the panic occurs in the template render and is swallowed by the template renderer.

This PR just adds some logging to force the panic to properly logged and re-propagates back up to the template renderer so we can actually detect what the issue is.

Signed-off-by: Andrew Thornton [email protected]
@6543 6543 added backport/done All backports for this PR have been created and removed skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Mar 23, 2021
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants