search

coleifer / micawber

a small library for extracting rich content from urls
http://micawber.readthedocs.org/
MIT License
635 stars 91 forks source link

added JSON error handling for nasty providers #30

Closed jvanasco closed 10 years ago

jvanasco commented 10 years ago

I recently came across an error with this page :

dailymotion should be returning an oEmbed packet, but instead returns the text "This video can not be embedded". i stress "text". it is not in json. it is just text.

this breaks the json load.

my proposed patch wraps the json load in a try/except block.

if an exception occurs (any, i'm not picky, though ValueError is trigged by this particular load ), it creates a new dict for the json data:

json_data = { 'micawber-error' : True, 'micawber-response' : response }

this allows people to easily inspect.

the url & title will be populated by the existing block.

an alternative would be to raise a ProviderException , or create a new exception:

class ProviderBadResponseException(ProviderException):
    pass

If that's the route you prefer, I would suggest stuffing the actual response into the exception .

I'll also add that the micawber demo on appsport will work on this particular url. it just seems to break on my machines. maybe this provider doesn't like me.

jvanasco commented 10 years ago

actually i'm going to switch to the ProviderBadResponse option. I did some more tests, and realized that JSON and SimpleJSON are raising different issues. json was giving me ValueError, SimpleJson was giving a decode error.

jvanasco commented 10 years ago

ok. The pull request is better now!

coleifer commented 10 years ago

Thanks for your ideas, I basically went w/a common exception type!

jvanasco commented 10 years ago

cool! thanks for the speedy fix on this.

also, apologies for the choice of bad urls. it was literally the only error like this that popped up in 2k parses of "previously bad urls" that I ran as a test this afternoon.