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

Properly catch NoUserException during upload cleanup #20284

Merged
merged 2 commits into from
Apr 4, 2020

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Apr 3, 2020

Follow up to #19655

getUserFolder might throw a NoUserException if the job was scheduled before the user removal. This would cause the background job to never succee and causing Backends provided no user object errors.

The second commit adds proper annotations for the exceptions being thrown by getUserFolder.

@@ -66,6 +67,9 @@ protected function run($argument) {
} catch (NotFoundException $e) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} catch (NotFoundException $e) {
} catch (NotFoundException|NoUserException $e) {

Does the same, without the duplication :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, fixed 👍

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Despite my nitpick this looks good :)

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl juliushaertl force-pushed the bugfix/noid/catch-no-user-in-cron branch from d424e0c to 56aa8fd Compare April 3, 2020 21:06
@juliushaertl juliushaertl added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 3, 2020
@rullzer rullzer merged commit 5c6f9ca into master Apr 4, 2020
@rullzer rullzer deleted the bugfix/noid/catch-no-user-in-cron branch April 4, 2020 08:25
@rullzer rullzer mentioned this pull request Apr 4, 2020
80 tasks
@juliushaertl
Copy link
Member Author

/backport to stable18

@juliushaertl
Copy link
Member Author

/backport to stable17

@juliushaertl
Copy link
Member Author

/backport to stable16

@backportbot-nextcloud
Copy link

backport to stable18 in #20334

@backportbot-nextcloud
Copy link

The backport to stable17 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable16 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants