Skip to content

Improve exception handling in Flodesk::Client #2321

@mikej

Description

@mikej

Currently we've got the following code in Client#request:

planner/lib/flodesk.rb

Lines 128 to 133 in 45fd9d1

rescue Faraday::Error => e
FlodeskError.new(e.response_body['message'], {
raw_body: e.response_body,
status_code: e.response_status
})
end

Make code nil-safe

This can result in a NoMethodError: undefined method '[]' for nil if there is an issue connecting to Flodesk and the response doesn't actually contain a message_body.

This could happen in production, but also can be a source of flaky tests as happened recently.

Proposing to make this nil-safe by using e.response_body&.['message']

Raise rather than return the exception

Currently this code is actually returning the FlodeskError rather than raising it.

Proposing to raise the exception instead, but note there's one place where we currently check the return type from the method that will need updating to reflect this change:

planner/lib/flodesk.rb

Lines 70 to 74 in 45fd9d1

def subscribed?(email:, segment_ids:)
response = request(:get, "subscribers/#{email}")
response => { status:, body: }
return false if response.is_a?(FlodeskError) && status == 404

Mock out the client in tests

As a separate task we can take a look at mocking Flodesk::Client in the specs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions