shrinerb / shrine

File Attachment toolkit for Ruby applications
https://shrinerb.com
MIT License
3.18k stars 275 forks source link

undefined method `[]' for nil:NilClass #508

Closed dssjoblom closed 3 years ago

dssjoblom commented 3 years ago

I'm running into a strange issue, where it seems like options are not passed correctly to plugins (?). This seems to apply to almost all of them, e.g. url_options and store_dimensions.

Expected behavior

That the code works normally on normal operations.

Actual behavior

This error can happen when doing almost anything, e.g. retrieving model.pic_url, where pic is the attachment. The message is undefined method `[]' for nil:NilClass when accessing options in a plugin. Here is another example:

begin
  rec.cover_pic = File.open( file ) if ok
rescue => e
  Utils.log_exception e, info: 'FAIL'
end
E, [2020-11-16T16:05:59.418508 #23534] ERROR -- : [acf054bb-3146-4eea-8c63-23e1ed1e47f7] undefined method `[]' for nil:NilClass
E, [2020-11-16T16:05:59.418681 #23534] ERROR -- : [acf054bb-3146-4eea-8c63-23e1ed1e47f7] /shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/store_dimensions.rb:75:in `extract_metadata'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine.rb:288:in `get_metadata'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine.rb:207:in `upload'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine.rb:108:in `upload'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/attacher.rb:182:in `upload'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/attacher.rb:117:in `attach'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/validation.rb:41:in `attach'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/attacher.rb:99:in `attach_cached'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/validation.rb:34:in `attach_cached'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/attacher.rb:77:in `assign'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/model.rb:114:in `model_assign'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/model.rb:41:in `block in define_model_methods'

I was able to accidentally resolve this in development (I saw the same problem, but then it disappeared for some reason), but now it is back when deploying to a remote server. If it matters, the background here is that I have migrated from Paperclip to Shrine, and moved attachment data with PaperclipShrineSynchronization (which seems to work, at least the attachment data is set, and the URLs work).

Additionally, in development, the problem only seems to be apparent when using the console, e.g. getting an attachment URL via the console, it works when uploading files via the app in the browser. On the remote server, nothing works. My guess is that it is some kind of initialization issue, but I'm unsure how to fix it.

System configuration

Ruby 2.7.2 Shrine 3.3.0

dssjoblom commented 3 years ago

The Uploader classes:

class ImageUploader < Shrine
  Shrine.plugin :store_dimensions

  Shrine.plugin :pretty_location

  Shrine.plugin :url_options, store: {
                  host: Rails.application.secrets.AWS_HOSTNAME
                }

  plugin :validation_helpers

  # Validate mime-type and size
  Attacher.validate do
    # Max 50MB
    validate_max_size 50*1024*1024

    # validate_mime_type %w[image/jpeg image/jpg image/png image/gif]
  end
end
require 'image_processing/mini_magick'

class BoardItemPicImageUploader < ImageUploader

  plugin :default_url

  Attacher.default_url do |derivative: nil, **|
    "/image/#{derivative}/missing.png" if derivative
  end

  plugin :derivatives, create_on_promote: true

  Attacher.derivatives do |original|
    magick = ImageProcessing::MiniMagick.source(original)
    {
      medium:  magick.resize_to_fit!(650, nil),
      thumb: magick.resize_to_fit!(200, nil),
    }
  end
end
dssjoblom commented 3 years ago

Another thing which may help with resolving this:

Case A works:

a = Shrine::Attacher.from_model BoardItem.first, :pic
a.url

=> correct URL

Case B doesn't:

b = BoardItemPicImageUploader::Attacher.from_model BoardItem.first, :pic
b.url

=> NoMethodError (undefined method `[]' for nil:NilClass)

Neither does case C:

begin
  BoardItem.first.pic_url
rescue => e
  puts e.message; puts e.backtrace
end

=>

undefined method `[]' for nil:NilClass
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/url_options.rb:22:in `url_options'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/url_options.rb:14:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/attacher.rb:253:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/default_url.rb:20:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/derivatives.rb:145:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/default_url.rb:20:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/derivatives.rb:145:in `url'
/shared/vendor/bundle/ruby/2.7.0/gems/shrine-3.3.0/lib/shrine/plugins/entity.rb:45:in `block in define_entity_methods'
dssjoblom commented 3 years ago

It seems like at least one issue here is calling Shrine.plugin instead of just plugin inside the uploaders. However, after changing this, I still can't generate URLs for :thumbnail or :medium without going through the Shrine base object, and it has none of the plugins installed:

2.7.0 :007 > b = Shrine::Attacher.from_model Board.last, :cover_pic
 => #<Shrine::Attacher:0x00007fc4df119dc8 @file=#<Shrine::UploadedF... 
2.7.0 :008 > b.url
 => correct URL, except host is wrong, because url_options is not in base class
2.7.0 :009 > b.url :thumb
 => correct thumbnail URL, but also with wrong host, for same reason
2.7.0 :010 > a = BoardCoverPicImageUploader::Attacher.from_model Board.last, :cover_pic
 => #<BoardCoverPicImageUploader::Attacher:0x00007fc4dd120f30 @file=#<BoardCoverPicImageUploader::UploadedFile storage=:store id="board/168/cover_pic/462cbe497cb401055cbf2a376181acf7.jpg" metadata=... 
2.7.0 :011 > a.url
 => correct URL, with correct host
2.7.0 :012 > a.url :thumb
 => "/image/thumb/missing.png" (if using default_url plugin) or nil (if not using default_url_plugin)

Even more info (after changing the Shrine.plugin calls to just plugin):

2.7.0 :013 > y Shrine.opts
---
:column:
  :serializer: !ruby/class 'Shrine::Plugins::Column::JsonSerializer'
:model:
  :cache: true
:activerecord:
  :callbacks: true
  :validations: true
:default_url: {}
:derivatives:
  :processors: {}
  :processor_settings: {}
  :storage: !ruby/object:Proc {}
2.7.0 :014 > y BoardCoverPicImageUploader.opts
---
:column:
  :serializer: !ruby/class 'Shrine::Plugins::Column::JsonSerializer'
:model:
  :cache: true
:activerecord:
  :callbacks: true
  :validations: true
:store_dimensions:
  :analyzer: :fastimage
  :on_error: !ruby/object:Proc {}
  :auto_extraction: true
:pretty_location:
  :identifier: :id
:url_options:
  :store:
    :host: https://dom9ca2w0lk5f.cloudfront.net
:validation_helpers:
  :default_messages:
    :max_size: !ruby/object:Proc {}
    :min_size: !ruby/object:Proc {}
    :max_width: !ruby/object:Proc {}
    :min_width: !ruby/object:Proc {}
    :max_height: !ruby/object:Proc {}
    :min_height: !ruby/object:Proc {}
    :max_dimensions: !ruby/object:Proc {}
    :min_dimensions: !ruby/object:Proc {}
    :mime_type_inclusion: !ruby/object:Proc {}
    :mime_type_exclusion: !ruby/object:Proc {}
    :extension_inclusion: !ruby/object:Proc {}
    :extension_exclusion: !ruby/object:Proc {}
:default_url:
  :block: !ruby/object:Proc {}
:derivatives:
  :processors:
    :default: !ruby/object:Proc {}
  :processor_settings:
    :default:
      :download: true
  :storage: !ruby/object:Proc {}
dssjoblom commented 3 years ago

Ok, so I finally found the solution to this. It seems like the PaperclipShrineSynchronization module also used Shrine.plugin:

Shrine.plugin :model
Shrine.plugin :derivatives

Removing this module completely (and fixing the other uses of Shrine.plugin) solves the problem. This issue can be closed, however, I would mention this in the documentation somewhere, as it is not very obvious how the plugins work without reading the source code of Shrine itself.

janko commented 3 years ago

Yes, plugins added to Shrine class won't work correctly in uploaders that have already subclassed Shrine before those plugins were loaded, because plugin options get copied over at the time of subclassing. I agree this should be clearly documented.

benkoshy commented 3 years ago

@janko

Amy trying to understand this particular problem. Take the following:

class ImageUploader < Shrine
  Shrine.plugin :store_dimensions
  plugin :another_plugin
end

In this particular case: store_dimensions will not be work for the ImageUploader because when we subclass Shrine, the store dimensions will not have been added (link here). Is this correct?

On the other hand, another_plugin will be added without issues because that will be called directly on self which would be the ImageUploader singleton class, and then going up the chain, the shrine singleton class (i.e. a class method). Is this correct?

So what is being suggested is that we must be careful in how we add plugins in Shrine subclasses?

janko commented 3 years ago

@BKSpurgeon When we call Shrine.plugin :store_dimensions, the plugin functionality will get added to ImageUploader (module inclusion on a superclass is applied to existing subclasses). However, that plugin's options will not be added to ImageUploader. This is because each subclass has its own copy of plugin options. That can break plugins that rely on keys existing. In this case, the store_dimensions plugin tried to read opts[:store_dimensions][:something], which broke because opts[:store_dimensions] was nil in ImageUploader.

On the other hand, another_plugin will be added without issues because that will be called directly on self which would be the ImageUploader singleton class, and then going up the chain, the shrine singleton class (i.e. a class method). Is this correct?

Just like adding plugins on Shrine won't expand to existing subclasses, adding plugins on subclasses won't get applied to superclasses, which includes the Shrine class. That actually stems from how inheritance works, including modules into subclasses won't include them on superclasses; this behaviour is the main reason we use inheritance in the first place. And plugin options won't get applied the same.