Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SendGrid Adapter to return ok/error tuple #572

Merged
merged 1 commit into from Feb 19, 2021

Commits on Feb 19, 2021

  1. Update SendGrid Adapter to return ok/error tuple

    What changed?
    ==============
    
    We update the `SendGridAdapter` to abide by the new Adapter behaviour.
    The adapter now returns an `{:ok, email}` or `{:error, error}`.
    
    As part of this work, we create the `build_api_error` function to
    replace the `raise_api_error` function.
    
    Note on implementation with throw/catch
    ---------------------------------------
    
    Building SendGrid's body requires a lot of deeply nested data
    manipulation. Nested deep within some of the branching logic was some
    code that raised errors.
    
    Rather than returning an error tuple at a really low level and have to
    bubble up the error all the way til it's returned, we opted for a
    simpler approach: throwing and catching the errors.
    
    We throw and catch `{:error, error}` when we used to `raise 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` and `catch`, but right now we want to minimize the number of
    changes for these `raise`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.
    germsvel committed Feb 19, 2021
    Configuration menu
    Copy the full SHA
    d5acd13 View commit details
    Browse the repository at this point in the history