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

Error handling logic, more options, small optimizations #20

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

tobek
Copy link

@tobek tobek commented Jul 14, 2014

No description provided.

Toby Fox added 9 commits July 2, 2014 19:52
Filed an issue in `request` module about this:
request/request#954

For some URLs (e.g. http://cnb.cx/1vtyQyv), `request` throws this error
on a HEAD request but not a GET request.
Looks like this chunk of code has been copied into the bitly function.
It’s no longer reachable: if (map['bitly'].active) and the host is in
the bitly map, the preceding for-loop would catch it. In addition, the
bitly function handles the absence of options[‘bitly’] correctly anyway
so no need to check for it.
Note that while using this option is slower than not using it, it is
roughly the same (same # of requests) as *not* using the API and just
using the generic method in the first place.
Instead of the original URL, because we may have successfully followed
original URL to a new URL and only errored out on that one.

We could also make this behavior optional.
The regex anyway breaks on unsual chars (like parentheses) in URLs

Also some tiny cleanup/optimization
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.

1 participant