Skip to content

Don't crash with outdated share provider on update with the web updater #34688

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.

Sign up for GitHub

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

Jump to bottom
Merged
szaimen merged 3 commits into from
Oct 20, 2022
Merged

Don't crash with outdated share provider on update with the web updater #34688

szaimen merged 3 commits into from
Oct 20, 2022

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 20, 2022

Fix #34673

@nickvergessen nickvergessen added this to the Nextcloud 26 milestone Oct 20, 2022
@nickvergessen nickvergessen requested review from a team, CarlSchwan, blizzz, juliusknorr and szaimen and removed request for a team October 20, 2022 06:50
@nickvergessen nickvergessen self-assigned this Oct 20, 2022
nickvergessen
nickvergessen commented Oct 20, 2022
View reviewed changes
@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 20, 2022

/backport to stable25

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 20, 2022
View reviewed changes
@nickvergessen nickvergessen force-pushed the bugfix/34673/dont-crash-with-outdated-share-providers branch from 736588d to a99afbf Compare October 20, 2022 07:19
@nickvergessen nickvergessen force-pushed the bugfix/34673/dont-crash-with-outdated-share-providers branch from a99afbf to 40d8ed9 Compare October 20, 2022 07:21
juliusknorr
juliusknorr reviewed Oct 20, 2022
View reviewed changes
@nickvergessen
Also catch in getProvider
9982909 
Signed-off-by: Joas Schilling <[email protected]>
szaimen
szaimen approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 20, 2022
@blizzz
Copy link
Member

blizzz commented Oct 20, 2022

Please run composer run cs:fix to format your code

@blizzz
Copy link
Member

blizzz commented Oct 20, 2022

Catching Throwable in all cases is broad. Maybe ok here. Mixed feelings.

@nickvergessen
Fix PHP CS
3dee71d 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 20, 2022

Catching Throwable in all cases is broad. Maybe ok here. Mixed feelings.

Same, but writing a regex to match that error message pattern and a broken update step sound worse :(

@nickvergessen
Copy link
Member Author

nickvergessen commented Oct 20, 2022

Please run composer run cs:fix to format your code

Fixed the unused import

@szaimen szaimen merged commit 9aaa370 into master Oct 20, 2022
@szaimen szaimen deleted the bugfix/34673/dont-crash-with-outdated-share-providers branch October 20, 2022 21:50
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Oct 20, 2022
Merged
@nickvergessen nickvergessen mentioned this pull request Oct 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@szaimen szaimen szaimen approved these changes

@blizzz blizzz Awaiting requested review from blizzz

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees

@nickvergessen nickvergessen

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: install and update

Projects

None yet

Milestone

Nextcloud 26

Development

Successfully merging this pull request may close these issues.

[Bug]: During Upgrade - PHP Fatal Error - Not compatible Declaration of RoomShareProvider::getSharesInFolder

5 participants

@nickvergessen @blizzz @juliusknorr @szaimen