#4554 render :text => proc { ... } regression - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4554 open

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

Reported by Jeremy Kemper | May 7th, 2010 @ 11:29 PM | in 3.0.6

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.

Comments and changes to this ticket

Jeremy Kemper August 30th, 2010 @ 04:10 AM
- Milestone cleared.
- Importance changed from “” to “Medium”
[bulk edit]
You flagged this item as spam.
Trotter Cashion September 14th, 2010 @ 01:08 AM
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.
- deprecate_render_text_with_proc.diff 2 KB
You flagged this item as spam.
Lake September 17th, 2010 @ 04:10 AM
Your patch applies cleanly to master and the tests are passing.

+1
You flagged this item as spam.
jrochkind September 27th, 2010 @ 04:21 PM
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.
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
Ryan Bigg October 19th, 2010 @ 08:34 AM
Automatic cleanup of spam.
You flagged this item as spam.
Rodrigo Rosenfeld Rosas October 21st, 2010 @ 07:28 PM
+1
You flagged this item as spam.
Rodrigo Rosenfeld Rosas October 21st, 2010 @ 07:35 PM
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...
You flagged this item as spam.
Kevin Menard October 30th, 2010 @ 11:27 PM
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.
You flagged this item as spam.
blaxter November 8th, 2010 @ 06:14 PM
Rodrigo, adding output.flush or output.write "Testing\n" you should get the response in the client without any wait.
You flagged this item as spam.
Martin Gogov November 9th, 2010 @ 11:14 AM
@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
You flagged this item as spam.
Martin Gogov November 9th, 2010 @ 02:06 PM
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
Santiago Pastorino November 15th, 2010 @ 09:04 PM
- Milestone cleared.
[bulk edit]
You flagged this item as spam.
John Firebaugh November 30th, 2010 @ 09:41 PM
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.
Santiago Pastorino February 12th, 2011 @ 09:58 PM
- Milestone set to “3.0.5”
[bulk edit]
Santiago Pastorino February 27th, 2011 @ 03:15 AM
- Milestone changed from “3.0.5” to “3.0.6”
[bulk edit]
You flagged this item as spam.
csnk May 18th, 2011 @ 08:25 AM
We are the professional scarfs manufacturer, scarfs supplier, scarfs factory, custom scarfs.
You flagged this item as spam.
Rodrigo Rosenfeld Rosas May 18th, 2011 @ 12:19 PM
Maybe we should close this now that 3.1 will support streaming again... What do you think?