Switch email separator from ";" to ","

[jcn]

According to the RFC, email addresses should be separated by a comma.

https://tools.ietf.org/html/rfc5322#section-3.6.3

This came up because I we have been trying to configure our server to send using SES and the server started crashing with the following error message:

2020-11-06 16:33:34 =SUPERVISOR REPORT====
     Supervisor: {local,gun_sup}
     Context:    child_terminated
     Reason:     {{owner_gone,{{nocatch,{permanent_failure,<<"554 Transaction failed: Illegal semicolon, not in group\r\n">>}},[{gen_smtp_client,try_DATA,4,[{file,"/lorawan-server/_build/default/lib/gen_smtp/src/gen_smtp_client.erl"},{line,333}]},{gen_smtp_client,send_it,2,[{file,"/lorawan-server/_build/default/lib/gen_smtp/src/gen_smtp_client.erl"},{line,193}]},{gen_smtp_client,send_it_nonblock,3,[{file,"/lorawan-server/_build/default/lib/gen_smtp/src/gen_smtp_client.erl"},{line,114}]}]}},[{gun,owner_gone,1,[{file,"/lorawan-server/_build/default/lib/gun/src/gun.erl"},{line,966}]},{gun,proc_lib_hack,5,[{file,"/lorawan-server/_build/default/lib/gun/src/gun.erl"},{line,649}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,247}]}]}
     Offender:   [{pid,<0.412.0>},{id,gun},{mfargs,{gun,start_link,undefined}},{restart_type,temporary},{shutdown,5000},{child_type,worker}]

This change should solve this problem.

Additional reference: https://forums.aws.amazon.com/thread.jspa?threadID=118409

[altishchenko]

Good one. Thanks!

[jcn]

@altishchenko the reason I found this in the first place was that the failure coming back from the SMTP server ended up killing the erlang process completely and I was unable to start it up again for long enough to actually change the setting. I would have thought that the server would be more resilient to an email sending failure like this, but apparently that particular error case isn't being handled properly.

[altishchenko]

@jcn I'd be more than happy to see the logs with the server being unable to start. Erlang is resilient to many sorts of failure and if the server doesn't start if means we have a problem somewhere else.

[jcn]

@altishchenko Thanks, I've opened issue #756 where we can discuss this further.

[jcn]

@gotthardp do you think it would be possible to do a patch release to get this (And other possible bug fixes) into the 0.6.x branch for those of us running a more stable platform?

[gotthardp]

And do you know what other fixes you want to get included?

[jcn]

And do you know what other fixes you want to get included?

@gotthardp The only thing that I think I'd personally be interested in would be the switch from Google Maps to Leaflet, just to remove the dependency, but that's not really a deal breaker for me right now.

I'm mostly just looking through the commits since 0.6.7 and there were a few things

• It looks like there was actually a version bump to 0.6.8 but that was never actually released
• 5df6557 and 7c8a649 haven't affected me but seems like they'd be reasonable candidates

For additional features in an 0.6.x branch, I feel like these would make sense, but I also respect not wanting to publish another 0.6.x feature branch.

• The CORS pre-flight would be useful to us as we are building on top of the API right now and currently need to ingest on our own instead of being able to call the server directly, but I know this is explicitly an 0.7.x feature so I understand not wanting to include that
• Same for prometheus stats