Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError, can't dump anonymous class #793

Closed
jess opened this issue Jul 24, 2012 · 18 comments
Closed

TypeError, can't dump anonymous class #793

jess opened this issue Jul 24, 2012 · 18 comments

Comments

@jess
Copy link

jess commented Jul 24, 2012

Think I found a bug.

Scenario: Only happens when using versions & using a session to store an object that has an uploader attached.

I'm using sessions for a cart. Something like this:

# app/models/cart
class Cart
  attr_reader :items

  def initialize
    @items = []
  end

  def add(product)
    @items << product
  end
end

# controller
product = Product.find(params[:id]) 
@cart = session[:cart] ||= Cart.new
@cart.add(product)

Now in my view:

@cart.items.each do |item|
  item.name
  image_tag item.image.thumb.url #causes the error...image.url will cause error too

Here's a short screencast showing the issue: http://screencast.com/t/GiP4tZwWY8S

Here's a test app you can run and get the result: https://github.com/jess/test_app
Note there's only a "wave" branch

Here's the trace: https://gist.github.com/3169779

I also ran into a similar issue with FriendlyID (maybe the cross reference will help??) norman/friendly_id#314

Please let me know if I can provide additional information...

@thiagofm
Copy link
Member

thiagofm commented Sep 4, 2012

I'm working on it and the test is very straightforward. Just try to Marshal.dump an instance of the mounter and we will have a TypeError raised.

However, I haven't found a way to rename the anonymous class yet(I'm studying a solution -- I can't get const_get to work right).

Also, isn't it better if you just add the id's of the Products in the session(instead of the whole object) and query them later in the database to show what's on the cart?

@thiagofm
Copy link
Member

thiagofm commented Sep 4, 2012

After spending a little more time on it. I still couldn't find a way to fix, but I'd like to give some words.

I've talked with some people from irc and just as I've thought, what you did is some kind of anti-pattern, as you shouldn't rely on the session to store the whole session object. BUT not being to marshal a model might be a problem if somebody tries to cache a model that has a carrierwave uploader. Or anything that plays with a model that needs it's marshaling.

Example: caching a Rails model using the Dalli gem(which is currently the most popular memcached gem) requires the marshaling of the object and if this object has a carrierwave uploader, it won't get marshaled. It won't get cached.

I'd like some input from the maintainers (help @bensie) into this issue before researching more deeply.

@jess
Copy link
Author

jess commented Sep 4, 2012

@thiagofm you're right, I believe it is an anti pattern...I ended up changing my code, as it caused problems all around.

Agree also that you should be able to cache the model/object.

Sorry if I'm incorrect here, but @norman corrected a very similar issue with friendly_id. I believe he used const_set to name the anonymous class. Could this approach work?

@thiagofm
Copy link
Member

thiagofm commented Sep 4, 2012

@jess I have tried for a while playing with const_set the way that the code is and couldn't figure a way to do it. Maybe somebody with a better understanding of the codebase can come up with a solution.

I have read some of the friendly_id code and their implementation seems different from ours.

If you have any articles that explain more deeply what @norman did to rename the anonymous class(I actually still don't really get what he did apart from using const_set), feel free to send me.

@bensie bensie closed this as completed Sep 4, 2012
@hrdwdmrbl
Copy link

AH! I finally track this error down to carrierwave.
My scenario: trying to render json using RABL. Also, trying to cache the RABL json. However, the issue is that when I try to call Marshal.dump on the hash which results from my model, it seems to contain

Some objects cannot be dumped: if the objects to be dumped include bindings, procedure or method objects, instances of class IO, or singleton objects, a TypeError will be raised.

My patch for this in my own code is to do (nvm, this patch blows up)

def thumbnail
  JSON.parse super.to_json
end

It's a crappy solution, but it allows the application to work. And, since I'm caching the render result, I don't actually have to run this each and every time. :)

p.s.
This should be re-opened.

@thiagofm thiagofm reopened this Oct 16, 2012
@thiagofm
Copy link
Member

Reopened. ✌️

So, @jackquack do you have any solution for it?

@hrdwdmrbl
Copy link

Not yet. I quickly found that my patch didn't actually work for anything other than rendering json.

I am curious about how the object is handled though. What method gets called when Marshal.dump is applied? (Marshal is written in c, so 👎 ) I'm not exactly clear how the Uploader object is handled by a call to to_json or as_json. Where does this anonymous class come from? I'm obviously not a ruby guru.

@thiagofm
Copy link
Member

@jackquack it's right here: https://github.com/jnicklas/carrierwave/blob/master/lib/carrierwave/mount.rb#L143

I couldn't find a way to name this anonymous class(maybe my const_set usage is wrong). As you are interested into it, I advise you to clone carrierwave, configure it to run the tests and play with it a little. Maybe from here we get into something.

What you did for doing this JSON.parse and .to_json is "filtering" the anonymous class, so this anonymous class never have the chance of being marshaled.

@dblock
Copy link

dblock commented Nov 3, 2012

As a workaround you can override the marshaller to exclude the mounted class.

    # only dump attributes when serializing a document
    def marshal_dump
      attributes
    end

    def marshal_load(data)
      instance_variable_set :@attributes, data
    end 

We've put this in Mongoid::Document to get it for all models, and it seems to just reduce the amount of data being marshaled without any adverse effect.

@bensie
Copy link
Member

bensie commented Jan 6, 2013

In the first reply, @thiagofm recommended that you store the cart ID in the session, not the object. This is absolutely correct.

Versioning creates anonymous classes by design -- the info about the upload is stored in a database column and I can't think of a valid reason that you need to be able to marshal Carrierwave upload objects.

@bensie bensie closed this as completed Jan 6, 2013
@hrdwdmrbl
Copy link

@bensie

The issue still exists for the marshalling of JSON in a caching solution.

@bensie
Copy link
Member

bensie commented Jan 6, 2013

Can you provide a more specific example? I am still failing to understand why marshaling the entire uploader object is necessary.

@bensie
Copy link
Member

bensie commented Jan 6, 2013

@jackquack I should have read in detail above, sorry. Caching a model that has a CW uploader mounted is valid. I shall have another look at this.

@bensie bensie reopened this Jan 6, 2013
@hrdwdmrbl
Copy link

@bensie Thanks!

I think I patched it for my own purposes by changing how RABL handles marshaling since that was the fix of least resistance. So, I can't offer any suggestions of my own as to what might be a good solution. :s

@bensie
Copy link
Member

bensie commented Jan 7, 2013

@jackquack @jess Can you please try master and let me know if this fixes the issue for you?

I've been unable to come up with a failing test (still working on it), but this should take care of the issue and wanted to get confirmation.

@bensie bensie closed this as completed in e306d0e Jan 8, 2013
@abonec
Copy link

abonec commented Jul 8, 2015

@bensie, this is don't work after load in new environment. Atomatic generated names for uploaders is missing with error like this:

undefined class/module ImageUploader::Uploader50028680 (ArgumentError)

@salanki
Copy link

salanki commented Apr 6, 2017

Exactly. They need a deterministic name. Reopen this issue or create a new one?

@mrsweaters
Copy link

I'm also experiencing this issue trying to cache Active Model Serializers via Dalli. I'd say just reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants