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

Protect against NPEs in notifications list (#10879) #10883

Conversation

zeripath
Copy link
Contributor

Backport #10879

Unfortunately there appears to be potential race with notifications
being set before the associated issue has been committed.

This PR adds protection in to the notifications list to log any failures
and remove these notifications from the display.

References #10815 - and prevents the panic but does not completely fix
this.

Signed-off-by: Andrew Thornton art27@cantab.net

Unfortunately there appears to be potential race with notifications
being set before the associated issue has been committed.

This PR adds protection in to the notifications list to log any failures
and remove these notifications from the display.

References go-gitea#10815 - and prevents the panic but does not completely fix
this.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.11.4 milestone Mar 29, 2020
models/notification.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 29, 2020
Co-Authored-By: Lauris BH <lauris@nix.lv>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 30, 2020
@zeripath
Copy link
Contributor Author

I guess that means that we will need to fix master branch similarly...

Funny that golangci-lint didn't catch it . Are we sure that golangci-lint is still running properly on master?

@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 30, 2020
@lunny
Copy link
Member

lunny commented Mar 30, 2020

Make L-G-T-M work.

@lunny lunny merged commit 596eebb into go-gitea:release/v1.11 Mar 30, 2020
@zeripath zeripath deleted the backport-fix-10815-panic-in-notifications-due-to-NPEs branch March 30, 2020 11:00
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants