Skip to content

Place wrapper around comment as diff to catch panics (#15085) #15086

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

Conversation

zeripath
Copy link
Contributor

This PR is a backport of #15085 on to 1.13

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]

@zeripath zeripath added this to the 1.13.5 milestone Mar 21, 2021
@zeripath zeripath changed the title Place wrapper around comment as diff to prevent panics (#15085) Place wrapper around comment as diff to catch panics (#15085) Mar 21, 2021
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Mar 21, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label 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 6543 merged commit b0819ef into go-gitea:release/v1.13 Mar 21, 2021
@zeripath zeripath deleted the comment-as-diff-panic-wrapper-1.13 branch March 21, 2021 15:17
@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
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants