Make RaiseError middleware configurable to not raise error on certain status codes (e.g. 404)

[clemens]

Description

This adds the possibility to use the RaiseError middleware while preventing certain status from raising errors.

A common example are cases where raising errors on 4xx/5xx status codes is generally desirable and consequently we want to use the RaiseError middleware and then having to work around a missing upsert (PUT) behavior by first trying to fetch a resource and depending on whether or not the resource is found, we subsequently POST to create or PATCH to update.

Before:

client = Faraday.new('https://example.com') do |b|
  b.response :raise_error
end

existing_object = begin
  client.get('/foo/1').body
rescue Faraday::ResourceNotFound
  # We expect this resource to potentially be missing, but still have to
  # rescue from it when using the RaiseError middleware
end

if existing_object
  client.patch('/foo', data)
else
  client.post('/foo', data)
end

After:

client = Faraday.new('https://example.com') do |b|
  b.response :raise_error, allowed_statuses: [404]
end

existing_object = client.get('/foo/1').body

if existing_object
  client.patch('/foo', data)
else
  client.post('/foo', data)
end

Closes #1587.

Todos

None.

Additional Notes

In addition to adding the feature, I've refactored the repetitive case/when statement. I'm happy to take it out if that change is undesirable.

[iMacTia]

This looks great, thank you for working on this!

I just want to be sure though you understand the behaviour will be slightly different from what you highlighted in the "After" section of the PR description:

# In the case of a 404 status, this won't raise an error anymore,
# which is what we want 👍
existing_object = client.get('/foo/1').body

# However the "failed" response will still be processed as a successful one,
# meaning `existing_object` will always evaluate to `true` here 👇
# (e.g. the body of the response could still be an empty string)
# 
# You'd need to use a more explicitly check here, e.g. on the response status
# or the actual presence of the response body
if existing_object
  client.patch('/foo', data)
else
  client.post('/foo', data)
end

This is perfectly fine for me and it does make sense, but appreciate it might cause a bit of surprise.
If you're OK with it, I can get this merged and released

[iMacTia]

neat!

[clemens]

Thanks for the positive feedback!

What you're saying is true: The code in my "After" example doesn't exactly work yet. I do think it makes more sense to adapt the example rather than trying to build in some "special" behavior into the middleware – would you agree? Then I'd just change the PR description accordingly.

[iMacTia]

Yes, absolutely 👍
No need to amend the PR description, it looks like we're on the same page anyway and the changes to the documentation don't present the same example.

I just wanted to make sure there was no misunderstanding 😄
Merging 🚀 !

[iMacTia]

This is now merged into main, do you think you might test the changes in your project from there @clemens?
Nothing against cutting a new release, just trying to play it safe with some testing if possible 🙂

[clemens]

This is now merged into main, do you think you might test the changes in your project from there @clemens? Nothing against cutting a new release, just trying to play it safe with some testing if possible 🙂

I've tested it today (with multiple 404 cases in a larger script) and everything worked as expected: I had other 4xx and 5xx errors being raised, but 404 just let the script proceed => great! :)