search

dustball / opencrisisline

Open Crisis Line - cloud-based open-source suicide & crisis help line
GNU General Public License v3.0
3 stars 1 forks source link

config.sample:logError cail to mail #14

Open twmoore3rd opened 4 years ago

twmoore3rd commented 4 years ago

Design philosophy debate: should a try/caught be wrapped around the mail function call in logError? Can be argued depends upon functionality outside of PHP working, mail can not be assumed to work. If an error in mail is detected, e.g., mail doesn't exist, errors out, etc.), should an error be written to a log file (a log file that doesn't exist yet).

Hmm, log file. Should logError both send email to the admin as well as write to a log file?

dustball commented 4 years ago

I don't think it needs try/catch. If the call fails, it simply returns false.

Also, there is a difference between the call creating an error that ends the program (which it does NOT do on failure, if I recall) vs. simply not sending mail.

Leaving the call in, best case it sends mail and worst case it fails not sending mail, right?

Or is this an issue about PHP generating warnings that show up to the user/browser?

I guess to simplify the discussion: what happened on your setup when mail() was left in?

twmoore3rd commented 4 years ago

Please don't try to KISS (Keep It Simple S*) on me. :-) Some humor I hope.

Yes, to simply/KISS: setup.php is a strange beast. setup.php only calls die as it is being used from the CLI. No reason to send email, right?

However setup.php includes config.php which in turn makes two database calls in try/catch constructs with the catch calling logAndDie that mails a warning and then calls die with the error message.

Okay, a question that I do not want to explore: an error has occurred so logAndDie is called. logAndDie then calls mail with the error text. What happens if mail doesn't exist on the system at this stage? I am thinking that all execution stops so the code never gets to the die which would the error text. The ultra safe approach would be to try/catch the mail and then die. The catch in this case would append the original error text with further text stating the mail isn't working.

And this ties into issue I just raised.

As the original code worked well enough, won't wrap the mail with a try/catch at this time.