App crashes when receiving exception from e-mail transport

bcksl3 years ago

7

• 

  jmikrut

  3 years ago

  That sounds right probably

  
  
  

  have you been able to sort out why your transport is throwing an exception?

  
  
  

  i kinda feel like this

  should

  happen

• 

  bcksl3 years ago

  Transient network failures, nothing out of the ordinary.

  
  
  @364124941832159242

  Thinking

  ECONNREFUSED

  shouldn't crash Payload if the mail server is down/unreachable.

• 

  Deleted User3 years ago

  Can we replace the built-in mailer and not only the transport?

• 

  jmikrut

  3 years ago

  could be possible

  
  
  

  i'd accept a PR to do that for sure

  
  
  

  as well as a silent failure if the mailserver crashes

  
  
  

  but i think that the default behavior would be to crash if mailserver crashes - - because technically the app's functionality is broken at that point (forgot pass, reset pass, etc)

  
  
  

  so to not crash on mailserver failure, we should do that only explicitly if a config prop is set to true

• 

  Deleted User3 years ago

  Cool. We are porting a Larvel app to Payload. We solved the problem using queues. If the remote mail server and sms gateway failed, the queue had a retry before giving up; that way, we could see who didn't receive a welcome, order confirmation, and other essential mail deliveries. When the mail service was up and running, we had the opportunity to run the failed queues again.

• 

  jmikrut

  3 years ago

  awesome!

  
  
  

  this sounds like a great add for payload honestly

• 

  bcksl3 years ago

  Mail delivery is an important component of account signups, password resets et al. but not to the extent that it should warrant a failure of the service entirely. The service in this state would be degraded, but primary functionalities such as login, API routes, and naturally the CMS itself would still be fully operational. I similarly wouldn't expect the app to crash if it were unable to retrieve an image from the S3 adapter, but rather to 404 or 500 as deemed most appropriate.

  
  
  

  Transient network failures including partition and congestion, as well as various failure modes of third party providers are unavoidable, and queuing is a fine addition to smooth things out. However, we must still ask ourselves the same questions. I imagine that, even when the retry attempts were exhausted, you would, e.g., write an entry to the database for later use, and not end the server process.

  @456226577798135808

  
  
  

  The doctrine is that the server should never crash due to entering a degraded state, so long as that state is transient, and especially when it is external.

  
  
  

  I would, on the other hand, have no issue whatsoever if the create operation for the auth collection were to return failure if email was unable to be delivered, but supposing this might cross a few abstraction boundaries or otherwise complicate something about the existing process, even failing silently has incredibly strong preference to crashing.

  
  
  @364124941832159242

  The most straightforward path that would put this in line with the current implementation would be failing the individual request (create user and friends), as is the case currently for when a DB connection drops.