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

Improved Error Handling #22

Closed
wants to merge 5 commits into from
Closed

Improved Error Handling #22

wants to merge 5 commits into from

Conversation

bre-17387639
Copy link

Resubmitting this due to a mistake I made in the branching. This builds off of this pull request here, so you will not want to merge both:

#15

@kennethreitz
Copy link
Collaborator

This seems like a logical way to handle things. I need to think about it further though :)

@idyedov
Copy link

idyedov commented Apr 24, 2013

+1 on better error/exception handling as right now there's no easy way to suppress it

@davecoutts
Copy link

And a +1 from me also.
I would like to see error handling brought under control within the grequests package.

At the moment I'm struggling with timeout error issues, as so.

# pip install cython==0.19.1
# pip install -e git://github.com/surfly/gevent.git@1.0rc2#egg=gevent
# pip install requests==1.2.1
# pip install grequests==0.2.0

import grequests

url = 'http://httpbin.org/delay/'
rs = (grequests.get('{0}{1}'.format(url, d), timeout=4) for d in (2, 4, 6))

for r in grequests.imap(rs, size=3):
    if r.ok:
        print r.url

.

Traceback (most recent call last):
  File "/home/dave/.virtualenvs/newgevent/src/gevent/gevent/greenlet.py", line 328, in run
    result = self._run(*self.args, **self.kwargs)
  File "/home/dave/.virtualenvs/newgevent/local/lib/python2.7/site-packages/grequests.py", line 128, in send
    return r.send(stream=stream)
  File "/home/dave/.virtualenvs/newgevent/local/lib/python2.7/site-packages/grequests.py", line 71, in send
    self.url, **merged_kwargs)
  File "/home/dave/.virtualenvs/newgevent/local/lib/python2.7/site-packages/requests/sessions.py", line 347, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/dave/.virtualenvs/newgevent/local/lib/python2.7/site-packages/requests/sessions.py", line 450, in send
    r = adapter.send(request, **kwargs)
  File "/home/dave/.virtualenvs/newgevent/local/lib/python2.7/site-packages/requests/adapters.py", line 333, in send
    raise Timeout(e)
Timeout: HTTPConnectionPool(host='httpbin.org', port=80): Request timed out. (timeout=4)
<Greenlet at 0x2c842d0: send(<grequests.AsyncRequest object at 0x2caa090>)> failed with Timeout

@juanriaza
Copy link

Any update?

@sbehrens
Copy link

Bump! This would be super helpful.

@Akibalogh
Copy link

+1

@juanriaza
Copy link

https://github.com/saghul/erequests now supports error handling

@dsully
Copy link

dsully commented Dec 17, 2013

+1

2 similar comments
@odcinek
Copy link

odcinek commented Dec 25, 2013

+1

@netllama
Copy link

+1

@kennethreitz
Copy link
Collaborator

#yolo

@kennethreitz
Copy link
Collaborator

@brendoncrawford can you rebase, and add some docs?

@ptwobrussell
Copy link

+1 for a way to handle exceptions raised within grequests.

@DanMcInerney
Copy link

+1

@SamJoan
Copy link

SamJoan commented Jun 3, 2014

Hey, would you like it if I rebased it and created a new pull request and added some docs? I can't merge changes into this pull request though, I don't think.

@ptwobrussell
Copy link

That would be terrific, thanks for offering to do that!

On Jun 3, 2014, at 5:25 AM, Pedro Worcel notifications@github.com wrote:

Hey, would you like it if I rebased it and created a new pull request and added some docs? I can't merge changes into this pull request though, I don't think.


Reply to this email directly or view it on GitHub.

@SamJoan
Copy link

SamJoan commented Jun 5, 2014

Hi there,

I've tried to merge, but there have been to many changes for me to succeed in that rebase.

Furthermore, there are 3 other pull requests similar to this one, sitting open :\

@ptwobrussell
Copy link

@droope - I'm not familiar enough with this code to try and evaluate the merits of the various pull requests and merge them myself, or I'd jump in at this point. I'm not sure what the other interim changesets are, but I may just fork from a previous revision where your patch is compatible and merge it in since this is the main changeset that I know that I need right now. I'm sure the other improvements are great, but until I know I need them, I'm not sure that they're going to help me. It's been almost two years since this issue was first identified, and I can't imagine how everyone else who uses this library doesn't need it as well. In almost any realistic scenario, it seems that you need to handle those exceptions.

@SamJoan
Copy link

SamJoan commented Jun 5, 2014

hi @ptwobrussell,

it's not my patch though, I was trying to merge it but it was originally created by @brendoncrawford

I agree with you regarding how essential these patches are. I've personally migrated to requests-futures, since that has error handling. and @kennethreitz (the author of this library) considers futures the way to go (#10)

Also, performance-wise, I've found the other library to be equal in speed or slightly faster, and doesn't require for every user of your application to compile gevent

@ptwobrussell
Copy link

@droope - Thanks for pointing me to https://github.com/ross/requests-futures

This looks great, and I wasn't yet aware of it. I'll take a look and see if I can't get moving with it. Really appreciate the link...

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.

None yet