search

dejan / rails_panel

Chrome extension for Rails development
https://chromewebstore.google.com/detail/railspanel/gjpfobpafnhjhbajcjgccbbdofdckggg
MIT License
3.85k stars 186 forks source link

Rails 7.1 compatibility issue #203

Closed abrom closed 1 month ago

abrom commented 3 months ago

hey folks, rather than re-opening #180 I thought it was best to start afresh. I've been doing some digging and have found the cause of the SystemStackError being raised when trying to using meta_request with Rails 7.1.x

In short it comes down to a change in the instrumentation mechanism to include locals:

https://github.com/rails/rails/commit/b451ff098de2b6505a3eeecd3f39c25ec8336904#diff-c92c886291bac7b41bab2a3a884a476ff8edfe2827da7db5ee0bf78c9ad8a17fR250

The problem being that meta_request tries to serialise the render payload to JSON, however the template property of the ActionView objects (if passed through the locals when rendering a template) has cyclic child dependencies (the routes) which can not be serialised.

The simple fix would be to exclude that key (locals) in the serialisation to revert the behaviour to pre-Rails 7.1.x as such: https://github.com/dejan/rails_panel/compare/master...Studiosity:rails_panel:v0.8.2

ie 

payload.except(:locals)

I don't see any negative impacts.. Happy to put together a PR.

abrom commented 3 months ago

Or potentially add ActionView::Helpers::FormBuilder (and others?) to not_encodable?

kbruccoleri commented 1 month ago

@abrom I think I stumbled on this too, I opened up https://github.com/dejan/rails_panel/pull/199/ but haven't gotten anywhere with merging it in.

I've encountered this bug again for possibly a different non-encodable class and your payload.except(:locals) solution seems to be more elegant. I think I'll update my PR to reflect that if you don't mind.

dejan commented 1 month ago

This should be fixed in 0.8.3.