Comments (8)
I came here to say the same thing, the error handling should be inside the method we provide IMO (in this case streaming_get
). I do think we should at least allow an error callback in case the stream raises an error somewhere halfway through that has nothing to do with the other code, like so:
sess.streaming_get('/get', on_error: &my_error_proc) do |stream|
if stream.status != 200
error_body = stream.receive_body
raise "Invalid status #{stream.status} - #{error_body}"
end
stream.each do |body_chunk|
# the fourth iteration of this will raise something
end
end
Where my_error_proc
is some user-definable proc that could do logging for example.
EDIT: Possibly we could also already put the error handling for non-200 responses in the streaming_get
method.
from patron.
This has (not-unexpectedly) again become relevant 😆 so I think I'll try a go at the following semantics:
- We get a method called
streaming_request
with a set of arguments, maybe similar to the existing#request
maybe slightly different. This is an opportunity to rethink the API surface a little bit - Since modern Rubies have block-level
rescue
I don't think having anon_error
callback is justified - unless we want to consider such a callback recoverable. Since most libCURL-originating errors are "terminal" for the running request (actually - I think all are) - raising from either the#streaming_request
method or the#each
method is appropriate. - For restart/recovery the caller should keep track on how much data they have already processed / received if they want
- To avoid mem bloat I am going to try doing an approach whereby
body_chunk
is a String allocated once and then reused, I believe the libCURL callback accepts the size of the buf as argument and will fill the buffer to the brim and no more, so this should be achievable. - At
#streaming_request
method end the libCURL request should forcibly close/terminate via an ensure, regardless whether thestream
has been read out in full or not
@WJWH @toland I will mention you from a PR once I have something
from patron.
@toland and anyone else - please do comment.
from patron.
Patron was originally intended to have a simple API. I wrote it because I really didn't like curb
's rather arcane API. Admittedly, curb
is a faithful representation of the libcurl
API, but that didn't feel very "Rubyish" to me.
The callback-based API just feels wrong. It directly messes with how we read and understand code. The term "callback spaghetti" isn't meant as a compliment 😄
The first and third examples both look good, but I would say that the first is more in keeping with the spirit with which I created Patron.
This is just my opinion. Take it with an appropriately sized grain of salt.
from patron.
Actually, the need for that resp.close
at the end of the first example bothers me. Folks are going to forget that and it will cause all kinds of badness. I think I would combine the first and third into something like this:
sess = Patron::Session.new
sess.streaming_get('/get') do |stream|
if stream.status != 200
error_body = stream.receive_body_and_close
raise "Invalid status #{stream.status} - #{error_body}"
end
stream.each do |body_chunk|
# do something with a body chunk
end
end
I like this approach because the stream
object is extensible; you aren't bound to the three parameters passed in the third example. Also, you can't forget to close the session like in the first example. Win, win!
It seems like you might also want methods like stream.to_file
and stream.to_string
in addition to stream.each
.
from patron.
And stream.to_socket
!
from patron.
Ok, imagine we do this. How do we ensure the stream gets closed correctly?
sess.streaming_get('/get') do |stream|
begin
if stream.status != 200
error_body = stream.receive_body
raise "Invalid status #{stream.status} - #{error_body}"
end
stream.each do |body_chunk|
# the fourth iteration of this will raise something
end
ensure
stream.close
end
end
from patron.
I am not sure I understand the question, so if what I am about to say seems out in left field, it probably is.
The stream is closed within #streaming_get
, whose implementation looks something like this:
def streaming_get(url) do
stream = setup_stream(url)
begin
yield stream
ensure
stream.close
end
end
(Please note that my Ruby is rusty, and this was done off the top of my head. Be kind 😄 )
Your example could then be simplified to:
sess.streaming_get('/get') do |stream|
if stream.status != 200
error_body = stream.receive_body
raise "Invalid status #{stream.status} - #{error_body}"
end
stream.each do |body_chunk|
# the fourth iteration of this will raise something
end
end
We will need to make sure that #streaming_get
returns an appropriate value when an error is raised. Or, since this function exists solely for its side-effects, maybe nil
is a reasonable return value. And nothing would stop a caller from using ensure
within the block if they want to handle certain exceptions themselves. We just want to make sure that the stream is closed when all is said and done.
from patron.
Related Issues (20)
- Release 0.8 HOT 1
- Automate the Patron release process HOT 5
- Idea: Connection pooling using curl_multi HOT 4
- Add streaming upload/download support HOT 1
- Release new version HOT 2
- The Hive copyright comment HOT 1
- Respond to signals when outside the GIL HOT 3
- Headers aren't parsed correctly for first request when connection is proxied HOT 1
- Redirect handling inconsistency HOT 3
- VCR issues after upgrading patron 0.10.0 -> 0.11.1 HOT 6
- PATCH method doesn't send body HOT 4
- Gem not installed to ruby 2.1.2 HOT 1
- how to set ssl client_key / client_cert ? HOT 6
- Request: Render full server response as a string HOT 1
- Figure out the actual set of supported HTTP verbs
- --http1.1 option please HOT 1
- "checking for ruby/thread.h... *** extconf.rb failed ***" when installing on google/cloud-sdk:alpine docker image HOT 1
- one rspec test fails on Arch Linux
- Basic Auth HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from patron.