Skip to content

Conversation

balvig
Copy link
Contributor

@balvig balvig commented Dec 12, 2023

What

Raises a custom ActiveResource::ConnectionError when failing to connect to an external endpoint completely, changing something like:

Errno::ECONNREFUSED: Failed to open TCP connection to 127.0.0.1:4568 (Connection refused - connect(2) for "127.0.0.1" port 4568)

...into:

ActiveResource::ConnectionRefusedError: Failed to open TCP connection to 127.0.0.1:4568 (Connection refused - connect(2) for "127.0.0.1" port 4568)

Why

Follows the convention of rescuing network errors and wrapping them in ActiveResource custom errors, allowing them to be rescued without leaking knowledge of internals.

@balvig balvig marked this pull request as ready for review December 12, 2023 01:48
@balvig balvig changed the title Raise ActiveResource::ConnectionError on Errno::ECONNREFUSED Raise ActiveResource::ConnectionRefusedError on Errno::ECONNREFUSED Dec 12, 2023
@rafaelfranca
Copy link
Member

This is a behavior change. If people are already rescuing ECONNREFUSED they would now get error in production. Can we do it without breaking existing code?

@balvig
Copy link
Contributor Author

balvig commented Jan 29, 2024

Cheers, good shout @rafaelfranca!

The only thing that immediately comes to mind is having the custom exception inherit from ECONNREFUSED rather than ConnectionError.

The lack of parity with the other custom exceptions feels a little off, but it might be worth it to avoid a breaking change?
Am I otherwise missing something? 🤔

@rafaelfranca rafaelfranca merged commit f07d6fe into rails:main Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants