@geekdenz opened this Pull Request on August 17th 2021 Contributor

fixes #17091

Description:

The lines changed may have already achieved this previously. I could not reproduce seeing any revealing details in the errors I provoked. However, it is now ensured that no detail of the exception is logged to the user and they just get told to contact the admin. The exception is logged to the system logs with the injected logger instance. Currently it is only using the implied __toString() method of the exception, which should provide adequate detail to admins. However, there might be better detail that could be logged here. Maybe we want to log the message with $ex->getMessage() instead like was previously fed back to the user.

Also, I noticed that Piwik::translate was used instead of using an injected Translator instance. @tsteur mentioned that we prefer to use DI, but there are a few instances of Piwik::translate in this file, so I left it for now, also because there it uses the injected class anyway and it might be easier to understand for a significant amount of users/developers.

Review

@geekdenz commented on August 23rd 2021 Contributor

When this error happens, please check your SMTP settings.

Also tell user where to change settings.

Catch the specific exception and show above error message.

@geekdenz commented on August 25th 2021 Contributor

No need to merge as solution is documentation. See issue.

This Pull Request was closed on August 25th 2021
Powered by GitHub Issue Mirror