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

Handling DNS errors #23

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

Handling DNS errors #23

wants to merge 3 commits into from

Conversation

klimli
Copy link

@klimli klimli commented Nov 1, 2017

When url returns DNS_PROBE_FINISHED_NXDOMAIN previous error handling doesn't work.
I left "as e:" on line 138 for further debugging process

When url returns DNS_PROBE_FINISHED_NXDOMAIN previous error handling doesn't work.
I left "as e:" on line 138 for further debugging process
One error handling is enough. Let's stick with this one.
@2hands10fingers
Copy link
Owner

@klimli Can I get a little more explanation on this before I merge? I'm not familiar with this error handling

@klimli
Copy link
Author

klimli commented Nov 2, 2017

Sure, it's mostly empirical though. Previous method failed when requests.get(url) failed to execute which was the case I encountered when it tried to download image from website that is no longer available. My internet browser when confronted with this url raised DNS_PROBE_FINISHED_NXDOMAIN error.
Here you can find different errors in request explained: docs

This discussion on stackoverflow is also helpful.

At first I thought that you will need 2 separate ways (yours original one and this one) to handle all errors but I tried it on real reddit data and it works fine with just this one.

Thanks for the Scraper!

@2hands10fingers
Copy link
Owner

@klimli Hey there! Thanks for getting back to me with a detailed response. I would work on something a little more like this

try:
    response = request.get(url)
except Exception as e:
    print("An error occurred .... {}".format(e))
    return -1
if response.status_code == 200:

I hope that makes sense. I essentially want to account for all the exceptions. Let me know if there are any questions.

@klimli
Copy link
Author

klimli commented Nov 4, 2017

Looks good.
Also, I found another error that is not managed properly:
#24

Should I remove pull request and you will just add this to the code?

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