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

fix(OC_Template.php): ensure we throw all errors #40763

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Psycojoker
Copy link

@Psycojoker Psycojoker commented Oct 4, 2023

Summary

In one branch of the try/catch combination, in one specific path no error was thrown at all, thus the user ended up with a 500 error page online but absolutely no error anyway in the logs.

Checklist

In one branch of the try/catch combination, in one specific path no error was thrown at all, thus the user ended up with a 500 error page online but absolutely no error anyway in the logs.

Signed-off-by: Bram <[email protected]>
@Psycojoker
Copy link
Author

Hello,

Do you want unit/other tests for this quick fix? I honestly would have absolutely no idea on how to do them, I have nearly 0 experience in php (and especially not modern php).

Kind regards and thx for this great software ❤️

@Psycojoker Psycojoker marked this pull request as ready for review October 4, 2023 01:42
@kesselb
Copy link
Contributor

kesselb commented Oct 4, 2023

If you reach line 316, that means no error occurred while logging the exception.
Are you sure there was nothing in your nextcloud logs?

@Psycojoker
Copy link
Author

Are you sure there was nothing in your nextcloud logs?

Yes, I've looked at every possible log imaginable:

  • nextcloud.log ofc
  • everything possible in /var/log/**/*.log
  • all fpm process, activating logs etc...
  • every possible related systemd process

Adding those lines was the only possible way to get the logging of the exact error somewhere (the error was that nextcloud couldn't connect to the database because it was stopped)

@kesselb
Copy link
Contributor

kesselb commented Oct 4, 2023

I assume we fixed that already for master/28 with #37458.

@Psycojoker
Copy link
Author

Maybe it would be a good idea to still have a fallback in the case the logging system broke for whatever reason again?

@solracsf solracsf added this to the Nextcloud 28 milestone Oct 27, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv skjnldsv added the 2. developing Work in progress label Feb 21, 2024
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants