Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

render :text => proc { ... } regression #675

Closed
lighthouse-import opened this Issue · 17 comments

2 participants

@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/4554
Created by Jeremy Kemper - 2010-10-31 05:57:56 UTC

render :text => someproc used to serve a streaming response. Now it calls to_s on the proc.

Should deprecate in favor of setting self.response_body = ... directly.

@lighthouse-import

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-08-30 03:10:32 UTC

[bulk edit]

@lighthouse-import

Imported from Lighthouse.
Comment by Trotter Cashion - 2010-09-14 00:08:24 UTC

We (Sean Grieve and myself) think we fixed this in the way you want. We made the patch on the 3-0-stable branch. If that's incorrect, let us know and we'll make it work on master.

@lighthouse-import

Imported from Lighthouse.
Comment by Lake - 2010-09-17 03:10:18 UTC

Your patch applies cleanly to master and the tests are passing.

+1

@lighthouse-import

Imported from Lighthouse.
Comment by jrochkind - 2010-09-27 15:21:03 UTC

I definitely need streaming response behavior in Rails3. If this is the patch neccessary to get it again, would be awesome if someone committed it.

@lighthouse-import

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-10-15 22:01:53 UTC

[bulk edit]

@lighthouse-import

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-19 07:34:44 UTC

Automatic cleanup of spam.

@lighthouse-import

Imported from Lighthouse.
Comment by Rodrigo Rosenfeld Rosas - 2010-10-21 18:28:02 UTC

+1

@lighthouse-import

Imported from Lighthouse.
Comment by Rodrigo Rosenfeld Rosas - 2010-10-21 18:35:12 UTC

Sorry, I need to be more specific. We need to remove the mention "Tip: if you want to stream large amounts of on-the-fly generated data to the browser, then use render :text => proc { ... } instead. See ActionController::Base#render for more information."

http://api.rubyonrails.org/classes/ActionController/Streaming.html

And we need some alternative to data streaming. Unfortunately, just using "self.response_body = proc {|response, output| 5.times{output.write 'Testing
'; sleep 1} }" won't flush, until the end, for instance.

So, the deprecation warning for the proposed patch is not correct enough yet...

@lighthouse-import

Imported from Lighthouse.
Comment by Kevin Menard - 2010-10-30 22:27:20 UTC

Rodrigo's point is very good. I'd go so far as to say the deprecation message is just flat out wrong. It's not deprecated if the expected behavior is already removed. And currently there isn't a real way to stream large amounts of generated data.

@lighthouse-import

Imported from Lighthouse.
Comment by blaxter - 2010-11-08 18:14:33 UTC

Rodrigo, adding output.flush or output.write "Testing\n" you should get the response in the client without any wait.

@lighthouse-import

Imported from Lighthouse.
Comment by Martin Gogov - 2010-11-09 11:14:42 UTC

@blaxter

I tried adding output.flush but I got an error undefined method 'flush' for #<ActionDispatch::Response ...>

Actually format and response seem to be the same object. Isn't this wrong?

I also tried output.write "Something\n" but it also didn't flush and returned the response at the end.

I used wireshark to sniff whether any packets are being sent meanwhile and there was no activity before the final response got sent back. I'm using Webrick but I read that I should try Mongrel to make this work so that's what I'm trying now.

Martin

@lighthouse-import

Imported from Lighthouse.
Comment by Martin Gogov - 2010-11-09 14:06:00 UTC

Sorry for the misformatted previous post.

Just wasted some more hours on checking this:

The self.response_body = proc { |resp, output| ... } approach with mongrel (webrick didn't work) worked almost fine for me (no need to output.flush, there's no such method in my case anyways).
I'm using ruby 1.9.2p35 with rails 3.

One side problem I still haven't worked around though is that with this approach the code in the proc gets executed twice so I have to put an if which skips every other execution to avoid double execution.

And still, the resp and output variables point to the same object. No idea why.

Martin

@lighthouse-import

Imported from Lighthouse.
Comment by Santiago Pastorino - 2010-11-15 21:04:51 UTC

[bulk edit]

@lighthouse-import

Imported from Lighthouse.
Comment by John Firebaugh - 2010-11-30 21:41:44 UTC

As Martin alludes to, the proc assigned to response_body is mistakenly executed twice. This is because ActionDispatch::Response overrides #body (defined in Rack::Response with attr_accessor), and enumerates the body. Thus when Rack::Response#close calls body.close, a second execution is triggered.

I think that ActionDispatch::Response should not override body. If it needs to force a string it should define #body_text or some other method with a name that makes it clear that it disables streaming.

On the subject of when or if response data is flushed, as far as I can tell it's entirely up to the Rack handler being used. I can confirm that Mongrel does flush automatically.

@lighthouse-import

Imported from Lighthouse.
Comment by Santiago Pastorino - 2011-02-12 21:58:21 UTC

[bulk edit]

@lighthouse-import

Imported from Lighthouse.
Comment by Santiago Pastorino - 2011-02-27 03:15:38 UTC

[bulk edit]

@josevalim josevalim closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.