When no mail providers are registered, fall back to no-op behavior #714
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #714 +/- ##
==========================================
- Coverage 35.14% 35.12% -0.03%
==========================================
Files 574 574
Lines 31176 31155 -21
Branches 930 929 -1
==========================================
- Hits 10958 10942 -16
+ Misses 20072 20067 -5
Partials 146 146 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I'd say this looks fine to me. Might be better if @jonashendrickx gave it a look as well.
| } | ||
| } | ||
|
|
||
| _logger.LogCritical(FallBackFailedMessage); |
There was a problem hiding this comment.
If none of the registered e-mail providers can send, we need to know all of them fail. Because at that point the customers could be critically impacted being unable to log in.
| return; | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _logger.LogError(e, "Failed to send message using provider '{Provider}'", providerConfiguration.Name); |
There was a problem hiding this comment.
This one was just to let us know when we fail to send using a specific provider, and log accordingly if it is not being logged by the provider itself specifically. It is to help us know when there is a specific issue with either a template or credentials of a specific provider.
| { | ||
| o.Providers = new List<BaseMailProviderOptions> | ||
| { | ||
| new FileMailProviderOptions() |
There was a problem hiding this comment.
Removing it might not be a good idea, we could define a new mail provider to log to the console using a warning or info log level for testing. Or output to Memorycache or some other in-memory option.
For self-hosting keeping the file is better by default.
There was a problem hiding this comment.
I think all you need to do to fix the API testing image is the following:
There was a problem hiding this comment.
It is ready, the tests are passing
#705 changed the behavior in regard to how the mail providers are configured:
FileMailProvideris used.The issue is that, in certain scenarios, the application may not be able to write data to the file system and thus may not use the
FileMailProvider, making it a poor default choice. This happens, for example, when running in Docker, where the application is executed under an under-privileged user account for security reasons:Additionally, there does not appear to be a way to set the
NoopMailProviderthrough configuration as it's only defined in tests.This PR reverts some of the user-facing behavior changes introduced in #705 and makes it so that no emails are sent when the configuration does not specify any mail provider.