We originally took heavy inspiration from Alertmanager's SMTP dispatch functionality due to the number of options we need to support.

In their dispatch code, they defer the call to message.Close but in fact this is necessary to complete the SMTP submission and await any errors; we do the same in our code.

NOTE: We use emersion/go-smtp as our SMTP library while Alertmanager uses net/smtp, but the former extends the latter, so the code that follows is effectively the same.

// coderd/notifications/dispatch/smtp.go
// Our client calls Data(), which returns a `dataCloser`.
message, err := c.Data()

...

// emersion/go-smtp@v0.21.2/client.go
func (c *Client) Data() (io.WriteCloser, error) {
	_, _, err := c.cmd(354, "DATA")
	if err != nil {
		return nil, err
	}
	return &dataCloser{c: c, WriteCloser: c.text.DotWriter()}, nil
}
...
func (d *dataCloser) Close() error {
	...
	if d.c.lmtp {
          ...
	} else {
		_, _, err := d.c.readResponse(250) // <---- this is the crucial line
		if err != nil {
			return err
		}
	}

	d.closed = true
	return nil
}

We cannot defer a call to Close because we depend on the error which is returned to determine if the message was successfully sent or not.

_, _, err := d.c.readResponse(250)

This line checks if the response was successful (docs); if not, we need to fail this dispatch and mark it for retry.

I discovered this bug by incorrectly configuring CODER_NOTIFICATIONS_EMAIL_FROM as my @coder.com address (which is backed by GSuite) and testing a dispatch via an outlook.com SMTP server. The server was rejecting the message with 554 5.2.252 SendAsDenied; <redacted>@outlook.com not allowed to send as <redacted>@coder.com, but the message was being marked successful, because we were not waiting for the actual outcome of the mail dispatch.

This has quite worrying implications for Alertmanager, and I plan to raise a PR in that repo too to address this.