Update SendGrid Adapter to return ok/error tuple #572
Merged
+42
−32
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.
Requires: #571
What changed?
We update the
SendGridAdapter
to abide by the new Adapter behaviour. Adapters must now returns an{:ok, email}
or{:error, error}
.Note on implementation with throw/catch
Building SendGrid's body requires a lot of deeply nested data manipulation. Some code that is deeply nested within branching logic raises errors.
Rather than returning an error tuple at a really low level and have to bubble up the error all the way until it's returned, we opted for a simpler approach: throwing and catching the errors.
We throw and catch
{:error, error}
when we used toraise error
.It's possible we could do something different in the future, like bubbling up the errors and returning them without having to resort to
throw
andcatch
, but right now we want to minimize the number of changes for theseraise
s deep in the call stack.Raising (not returning) configuration errors
While working on this, an important question came up. Should we raise or return an error because of a missing API key? Currently we raise.
I think it's okay to raise an error in that case because it's a configuration issue. It's not the same type of error that happens when you can't build an email, deliver it, or even get a response that isn't 200. That error is a configuration issue and so the sooner we notify the user of Bamboo, the better.
We can also think about it this way: we want to return an ok/error tuple because we people want to handle email building or delivery issues. Having an API key missing isn't that kind of issue. It means the library is configured incorrectly, so it seems good to raise an error.