Fix/ux messages when smtp is not configured in backend

[weidmaster]

Problem

• The errors thrown by Swift SMTP class were very cryptic to non-developers using backend features that require sending e-mails.

Solution

• Following the philosophy of Winter CMS to be an user-friendly, out-of-the-box working for non-tech users, now there are try-catch blocks to be able to capture the Swift exceptions and then have the opportunity to show a more user-friendly oriented message, that also provides how to solve the SMTP issue at hand.

Sreenshots

before

after

Final considerations

• Other language files must be updated to reflect the new strings
• It should be able to manipulate the modal, to add buttons, for instance, to redirect to the SMTP configuration page. That would provide a better user flow from exceptions and error messages

[bennothommo]

@weidmaster I'm happy for this change to be merged, as long as the original exception is still logged so that a developer is able to diagnose the issue. Can you make sure the exception is still logged, or add in some log calls if not?

[LukeTowers]

There's some tweaks to the phrasing I'd like to make before merging @bennothommo

[weidmaster]

@weidmaster I'm happy for this change to be merged, as long as the original exception is still logged so that a developer is able to diagnose the issue. Can you make sure the exception is still logged, or add in some log calls if not?

I will double check if the logs are still there. As far as I know, all exceptions are logged by Winter, but I didn't check inside the admin backend log viewer. Thanks for remiding me.

[weidmaster]

There's some tweaks to the phrasing I'd like to make before merging @bennothommo

Sure, I have issues how to put the localization strings at the best form possible, or which array key was ideal. Please tell me how to proceed, if I am the one that needs to make the changes

[weidmaster]

There's some tweaks to the phrasing I'd like to make before merging @bennothommo

Sure, I have issues how to put the localization strings at the best form possible, or which array key was ideal. Please tell me how to proceed, if I am the one that needs to make the changes

@bennothommo I am trying to make the log appear in the System Event page, but to no avail. I read all the docs about Excepton handling, throwing, event firing, all I could but I am not able to save the relevant parent exception to the database!

I have tried the following code, that does indeed log the correct parent exception with the stack trace, but the Event is not firing at all, ie, no entry in system_events_logs is made. I need some help with this.

try {
  $this->sendInvitation();
} catch (Swift_TransportException $e) {
  trace_log($e);
  Event::fire('exception.report', [$e]);
  throw new ApplicationException(Lang::get('backend::lang.account.send_invitation_error'));
}

Also, there is no current log in place for the recover password function, so making one work will be implemented to the other one.

[bennothommo]

@weidmaster I've just added a commit which should do the logging and show up in the Event logs. Give that a try and let me know if it works for you.

[weidmaster]

@weidmaster I've just added a commit which should do the logging and show up in the Event logs. Give that a try and let me know if it works for you.

I checked your commit. It is the same behaviour as using the helper trace_log($e) It adds an entry in logs/system.log but still does not add the entry to the database :sweat_smile:

I checked a bit further the stack trace going to the Swift exception but what I got is that Winter CMS is handling the exception.

From what I noticed, I think the better approach to the logging in this case would be to make changes to the trace_log() helper. It should accept a third parameter, that would indeed manipulate and create the entry in the database. What are your thoughts @LukeTowers ?

[LukeTowers]

My thoughts are that we should just avoid all of the extra complexity added here and just add extra handling to the system error handler to adjust the message that's reported to provide common solutions. Worth noting that there's other reasons that these exceptions can be thrown apart from a misconfiguration, you could also have invalid email addresses being provided among other issues.

[weidmaster]

My thoughts are that we should just avoid all of the extra complexity added here and just add extra handling to the system error handler to adjust the message that's reported to provide common solutions. Worth noting that there's other reasons that these exceptions can be thrown apart from a misconfiguration, you could also have invalid email addresses being provided among other issues.

I am all for simple processes too. Yes, there are other reasons the exceptions can be thrown, so UX needs to be treated when they are discovered. The System Exception Handler should be able to adjust the message, but the simple workflow of exception handling with try...catch blocks will be necessary for obscure exceptions, like this relay one.

The idea is just to fix displaying obscure messages to the end-user. If a third-party exception message is informative well enough to help the end-user make a decision, that won't need to be modified.

Thinking now, the most simple approach would be able to capture the original exception, just modify the message, and rethrow the original exception. So then we could show something along the lines of There was a problem while sending the invitation e-mail to this admin user. Please check the System Event Log for details.

This way, we can come up with some automation that will modify the "while" portion (what caused the issue) and then the end-user will always receive the message after to check the Log anyways, that will have the original exception messages.

[github-actions[bot]]

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this is still being worked on, please respond and we will re-open this pull request. If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[github-actions[bot]]

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this is still being worked on, please respond and we will re-open this pull request. If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[mjauvin]

@weidmaster @LukeTowers @bennothommo What is this all about, we're not using Swift anymore (moved to Symfony Mailer with Laravel 9) ?

[LukeTowers]

The error messages would be similar @mjauvin, this PR is just to transform opaque and unhelpful core mailer error messages into more actionable advice.

Reviewing it more though I'm not convinced this is a good idea. Having the specific error present is a lot easier to track down the issue and debug it rather than have a single error message to cover the entire gamut of things that can go wrong with email configuration & sending.

[github-actions[bot]]

This pull request will be closed and archived in 3 days, as there has been no activity in this pull request for the last 6 months. If you intend to continue working on this pull request, please respond within 3 days. If this pull request is critical for your business, please reach out to us at [email protected].