altmetric / embiggen

A Ruby library to expand shortened URLs
https://rubygems.org/gems/embiggen
MIT License
124 stars 6 forks source link

Remove expand! by raising errors during expand #10

Closed mudge closed 9 years ago

mudge commented 9 years ago

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:

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 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).

jbilbo commented 9 years ago

looks good to me :+1:

jakubpawlowicz commented 9 years ago

Otherwise, looks good to me!