search

edicl / drakma

HTTP client written in Common Lisp
http://edicl.github.io/drakma/
248 stars 58 forks source link

http-request connection-timeout parameter #61

Open ofosos opened 9 years ago

ofosos commented 9 years ago

Hi Edi, I just wondered why the documentation states that the parameter connection-timeout of http-request is the time drakma waits for a server to respond initially (that's how I read it). From the usocket documentation for socket-connect and its :timeout parameter over at https://common-lisp.net/project/usocket/api-docs.shtml this sets SO_RCVTIMEO which does have the effect of setting a timeout for connect() but it also determines the timeout for read() (but not for write()).

I'll be testing this tomorrow, since I need a timeout on drakma. Do you concur with my above description? May I submit a pull request to explain this a little more in the documentation?

hanshuebner commented 9 years ago

Mark,

I do not concur with your reading of the description. The documentation should be accurate. The three timeout parameters are independent of each other, read-timeout and write-timeout are available only on LispWorks. If in doubt, consult the source.

-Hans

2015-07-06 20:34 GMT+02:00 Mark Meyer notifications@github.com:

Hi Edi, I just wondered why the documentation states that the parameter connection-timeout of http-request is the time drakma waits for a server to respond initially (that's how I read it). From the usocket documentation for socket-connect and its :timeout parameter over at https://common-lisp.net/project/usocket/api-docs.shtml this sets SO_RCVTIMEO which does have the effect of setting a timeout for connect() but it also determines the timeout for read() (but not for write()).

I'll be testing this tomorrow, since I need a timeout on drakma. Do you concur with my above description? May I submit a pull request to explain this a little more in the documentation?

— Reply to this email directly or view it on GitHub https://github.com/edicl/drakma/issues/61.

ofosos commented 9 years ago

You're correct despite the documentation saying so, usocket does not set a proper read timeout. I just took the read timeout code for sbcl from hunchentoot and enabled the read-timeout parameter in drakma for sbcl. I'll be testing that till tomorrow.

hanshuebner commented 9 years ago

2015-07-08 6:14 GMT+02:00 Mark Meyer notifications@github.com:

You're correct despite the documentation saying so, usocket does not set a proper read timeout. I just took the read timeout code for sbcl from hunchentoot and enabled the read-timeout parameter in drakma for sbcl. I'll be testing that till tomorrow.

It is not usocket that does not set a "proper" timeout in this case, but rather drakma. Only connect timeouts are supported with usocket. Read and write timeouts are LispWorks only. If you want to make a change in that area, please make sure that you're properly updating the documentation as well.

The deadline option on CCL is known to work well.

-Hans

ofosos commented 9 years ago

Hi Hans, as an experiment I modified the code to set a read timeout, however this signals the condition sb-sys:io-timeout. It feels odd to let implementation defined conditions to leak out of drakma, should I handle that condition and resignal a drakma defined condition?

hanshuebner commented 9 years ago

I don't think that Drakma should offer a way to set timeouts without also offering a portable way to catch timeouts. Maybe a better approach would be to offer a hook that Drakma calls once the socket is opened. In that hook, one could set timeouts in whatever way the implementation offers.​

ofosos commented 9 years ago

That seems sensible. Either provide a hook to set the implementation defined parameters of the socket or push down :read-timeout and :write-timeout into usocket. I'll be looking into the latter option tomorrow. I'll also have to ask the usocket people, under what conditions they would accept that addition.

ofosos commented 9 years ago

Sometimes I feel like stumbling through the world blindfolded.... https://github.com/usocket/usocket/blob/master/option.lisp usocket has a socket option implementation, which serves this purpose. The conditions are signalled portably. I'll give that a try.

avodonosov commented 7 years ago

As of now, usoket:socket-connect on CCL passes the timeout both to 

                  :connect-timeout ,timeout
                  :input-timeout ,timeout
otwieracz commented 7 years ago

+1 here, connection-timeout should also become available in CCL.