search

steveklabnik / request_store

Per-request global storage for Rack.
https://github.com/steveklabnik/request_store
MIT License
1.47k stars 87 forks source link

Make compatible with Rails streaming #52

Closed aasmith closed 6 years ago

aasmith commented 8 years ago

When rendering a view with render stream: true the middleware clears the request store prematurely. This is because the request is considered complete shortly after the controller returns and not when all of the view fragments have been rendered. By wrapping the response in a Rack::BodyProxy, the middleware will instead clear out the request store after all of the streamed view is rendered and the body is complete.

I'm not sure if this is a common enough use case to accommodate in this gem, but I thought it might be useful to have the discussion, as well as help anyone else who may be seeing this behavior.

Fivell commented 8 years ago

@aasmith , I was seeing this behaviour, and described it here https://github.com/steveklabnik/request_store/pull/25#issuecomment-160755460

More than that if you add something to request store in streamed view it will be not cleanup and next request will contain all "thread-local" variables

aasmith commented 8 years ago

I believe this patch addresses the timing as to when the end of the request really occurs. However, if you are working with the request store inside one of the view fragments generated during view streaming, you have more work to do. The fragments are rendered inside a Fiber, which gets its own Thread.current. You would still need to pass the items you need from the existing Thread.current request store on to this new fiber, however that is outside of the scope for this patch/project maybe? This caveat only applies to view streaming, AFAIK.

macfanatic commented 6 years ago

Is there any plans to support merging this into master?

We're using this in production to fix an issue with streaming CSV downloads in ActiveAdmin, as referenced in https://github.com/activeadmin/activeadmin/issues/5115

Thanks!

steveklabnik commented 6 years ago

I haven't looked at request_store, in a while, but if this is an important patch for ActiveAdmin, I'm happy to :shipit: . Give me a bit.

steveklabnik commented 6 years ago

Released as 1.4.0. Thanks everyone!

macfanatic commented 6 years ago

Thanks a ton!