This repository was archived by the owner on Sep 30, 2024. It is now read-only.
feat/graphql/access_tokens: send access token creation/deletion emails in background and create special message for dial errors#63760
Merged
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
6eb016d to
f590c83
Compare
bobheadxi
approved these changes
Jul 10, 2024
bobheadxi
left a comment
Member
There was a problem hiding this comment.
Nice, looks like it might be worth making a helper function for this? The two new blocks of code looks the same, docstrings and all
eseliger
reviewed
Jul 10, 2024
Contributor
Author
|
I think it's worth keeping this separte for a few reasons: Say that we have a siguratute that looks liek: func sendAccessTokenCreationEmail(ctx context.Context, db database.DB, logger log15.Logger, userID int32, note string, isDeleted bool
For these reasons, I don't think it's worth it to DRY this functionality yet. |
eseliger
approved these changes
Jul 10, 2024
…n emails in background and create special message for dial errors
f590c83 to
fbb493c
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Closes https://linear.app/sourcegraph/issue/SRC-463/error-handling-for-smtp-config-issues
Before, when someone created or deleted an access token, we'd block on sending the email before we'd return to the user.
However, if an error occurred when sending an email - the error was only logged - not returned to the caller.
This PR makes three changes:
We now sending the email in a background goroutine. This makes it so that the caller doesn't have to wait for the email to be sent before getting the access token back. This isn't a behavior change since we only logged any SMTP errors and deliberatly avoided forwarding that error to the caller. That behavior doesn't change here.
If an error occurs while sending the access token email, we now have a specialized log message indicating that the user's SMTP configuration might be incorrect.
The log error that's printed is now an
ERRORinstead of aWARNlevel.Test plan
On local dev, I edited the credentials in the dev-private site configuration file to point to an invalid port.
I attempted to create an access token, and after only 30 seconds I saw this error print in the terminal:
Changelog