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

Allow other HTTP clients to be used #9

Open
mudge opened this issue May 27, 2015 · 2 comments
Open

Allow other HTTP clients to be used #9

mudge opened this issue May 27, 2015 · 2 comments

Comments

@mudge
Copy link
Contributor

mudge commented May 27, 2015

Embiggen currently uses Ruby's standard Net::HTTP library to perform HTTP requests but this is not open for extension (e.g. to configure proxies or swap out entirely). It might be better to provide an adapter interface for other HTTP client libraries.

mudge added a commit that referenced this issue May 31, 2015
GitHub: #1

Rather than having separate expand and expand! methods (one swallowing
all exceptions, one not), simplify the API of Embiggen::URI to a single
expand method. This no longer aggressively swallows exceptions outside
of its control but may raise subclasses of Embiggen::Error, namely:

* Embiggen::BadShortenedURI;
* Embiggen::TooManyRedirects;
* Embiggen::NetworkError.

The last of these wraps any network-related exceptions including
timeouts, connection resets, unreachable hosts, etc.

The reason for this change is that these truly are exceptional
circumstances outside of the library's control (unlike, say, an
unshortened URI being passed to Embiggen::URI which can be handled
gracefully). The previous API took too much responsibility away from the
client by silently discarding exceptions by default and therefore
obscuring failures.

Originally, the idea of using a [sum type][0] was appealing (e.g.
returning some sort of result object which could be interrogated for its
success) but this later seemed overengineered and unidiomatic. Instead,
we prefer to return standard Ruby types where appropriate (e.g. standard
library URIs) and raise exceptions in case of failure.

This simplification lead to a refactoring that extracted an HttpClient
class which is currently used privately within the library. This
potentially opens the way for pluggable HTTP clients (rather than being
forced to use Net::HTTP) in future (c.f. #9).

[0]: http://en.wikipedia.org/wiki/Tagged_union
mudge added a commit that referenced this issue May 31, 2015
GitHub: #1

Rather than having separate expand and expand! methods (one swallowing
all exceptions, one not), simplify the API of Embiggen::URI to a single
expand method. This no longer aggressively swallows exceptions outside
of its control but may raise subclasses of Embiggen::Error, namely:

* Embiggen::BadShortenedURI;
* Embiggen::TooManyRedirects;
* Embiggen::NetworkError.

The last of these wraps any network-related exceptions including
timeouts, connection resets, unreachable hosts, etc.

The reason for this change is that these truly are exceptional
circumstances outside of the library's control (unlike, say, an
unshortened URI being passed to Embiggen::URI which can be handled
gracefully). The previous API took too much responsibility away from the
client by silently discarding exceptions by default and therefore
obscuring failures.

Originally, the idea of using a [sum type][0] was appealing (e.g.
returning some sort of result object which could be interrogated for its
success) but this later seemed overengineered and unidiomatic. Instead,
we prefer to return standard Ruby types where appropriate (e.g. standard
library URIs) and raise exceptions in case of failure.

This simplification lead to a refactoring that extracted an HttpClient
class which is currently used privately within the library. This
potentially opens the way for pluggable HTTP clients (rather than being
forced to use Net::HTTP) in future (c.f. #9).

[0]: http://en.wikipedia.org/wiki/Tagged_union
mudge added a commit that referenced this issue Jun 1, 2015
GitHub: #1

Rather than having separate expand and expand! methods (one swallowing
all exceptions, one not), simplify the API of Embiggen::URI to a single
expand method. This no longer aggressively swallows exceptions outside
of its control but may raise subclasses of Embiggen::Error, namely:

* Embiggen::BadShortenedURI;
* Embiggen::TooManyRedirects;
* Embiggen::NetworkError.

The last of these wraps any network-related exceptions including
timeouts, connection resets, unreachable hosts, etc.

The reason for this change is that these truly are exceptional
circumstances outside of the library's control (unlike, say, an
unshortened URI being passed to Embiggen::URI which can be handled
gracefully). The previous API took too much responsibility away from the
client by silently discarding exceptions by default and therefore
obscuring failures.

Originally, the idea of using a [sum type][0] was appealing (e.g.
returning some sort of result object which could be interrogated for its
success) but this later seemed overengineered and unidiomatic. Instead,
we prefer to return standard Ruby types where appropriate (e.g. standard
library URIs) and raise exceptions in case of failure.

This simplification lead to a refactoring that extracted an HttpClient
class which is currently used privately within the library. This
potentially opens the way for pluggable HTTP clients (rather than being
forced to use Net::HTTP) in future (c.f. #9).

[0]: http://en.wikipedia.org/wiki/Tagged_union
@dentarg
Copy link

dentarg commented Jan 5, 2016

I guess this, if implemented, would expose the ability to change the User-Agent used by embiggen, right?

@mudge
Copy link
Contributor Author

mudge commented Jan 5, 2016

Yes: if we allowed users to access (and even provide) the HTTP client itself, then they could configure it however they wish (e.g. proxy settings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants