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

Serialize Object/Array when no AMS Serializer is defined #945

Closed
kayakyakr opened this issue Jun 9, 2015 · 23 comments
Closed

Serialize Object/Array when no AMS Serializer is defined #945

kayakyakr opened this issue Jun 9, 2015 · 23 comments

Comments

@kayakyakr
Copy link

I'm receiving an issue, in latest master, where I receive NoMethodError (undefined method new' on nil:NilClass.)` when I attempt to serialize something like, ['Result is invalid'].

From everything I can tell, it's rolling through the get_serializer_for method, failing to find a serializer for my object, but trying to create an instance of that serializer anyway. It should fall back to a default serializer.

Sorry if this has been addressed, I checked through the issues but didn't see anything that stood out to me. As a workaround, I'm having to specify "render json: obj, adapter: false" when I want to render a json-ready hash rather than a rails model.

@bf4
Copy link
Member

bf4 commented Jun 11, 2015

Needs more info. What is obj? Which ref are you on? (paste output of bundle show active_model_serializers) Which Ruby? Which Rails? What does your serializer look like? Could you provide some more of the stack trace?

@kayakyakr
Copy link
Author

active_model_serializers-7b0a85fdda85
rbx 2.2.10
rails 4.1

code is called as:

render json: {error: 'Result is Invalid'}
# throws exception because serializer is nil

Stack trace is less than interesting and I don't currently have an example set up to cause the error, but digging through the code:

#action_controller/serialization.rb:23
@_serializer ||= ActiveModel::Serializer.serializer_for(resource) # @_serializer is nil when a serializer for the resource is not explicitly defined.

#action_controller/serialization.rb:47
object = serializer.new(resource, @_serializer_opts) # calls new on serializer when it may be nil

Is render json: {} supposed to fall back to a more appropriate renderer?

@bf4
Copy link
Member

bf4 commented Jun 11, 2015 via email

@kayakyakr
Copy link
Author

I like my adapter: false workaround.

-- edited because it came across assholish, when I didn't mean it to be.

@bf4
Copy link
Member

bf4 commented Jun 12, 2015

I suppose we could argue this is a bug in AMS in that if it can't find a serializer for non-string object, it should either just pass it through untouched or call to_json on it depending on the context, since that is what the renderer does it non-string, eventually.

@kayakyakr
Copy link
Author

That would be the best, I think.

I also noticed that if you pass it a model that you haven't built a serializer for (yet or at all), it fails in the same way. Previous versions of AMS either passed those objects on to a lower renderer or just called to_json on them. Also chokes in the same way with an association without a serializer.

@bf4
Copy link
Member

bf4 commented Jun 12, 2015

Yeah, wanna pair on a PR to pass through if adapter is nil or serializer is nil?

@kayakyakr
Copy link
Author

I need to do some more digging. It looks like it should be passing it to super as soon as serializer comes back nil, which means something else is going on.

@bf4
Copy link
Member

bf4 commented Jun 12, 2015

I think you have it on the nose, if I understand what you understand. tl;dr

def _render_with_renderer_json(resource, options)
  if use_adapter? && (serializer = get_serializer(resource))
    # adapter ~ Adapter.create(serializer.new(resource))
     super(adapter, options)
  else
    super(resource, options)
  end
end
  • where use_adapter? is true either when :adapter is not given or when it's given and falsy
  • where get_serializer(resource) returns a serializer class either when :serializer is given or when it can be inferred by ActiveModel::Serializer.serializer_for(resource)
  • thus, the AMS adapter is used when the adapter is given or inferred AND the serializer is given or inferred.

So, it's surprising that render json: {error: 'Result is Invalid'} would hit object = serializer.new(resource, @_serializer_opts) since though use_adapter? is true, get_serializer should be returning nil

@kayakyakr
Copy link
Author

Yeah, that's what is confusing me. Let cause this error to occur and see what happens.

@bf4
Copy link
Member

bf4 commented Jun 12, 2015

So, that's an easy test to write, no?

def render_object_without_serializer
  render json: {error: 'Result is Invalid'}
end
def test_render_object_without_serializer
  get :render_object_without_serializer
  assert_equals @response.body, '{ "error": "Result is Invalid" }'
end

@kayakyakr
Copy link
Author

I got it!

render json: ['errors']

It's because it's wrapped in an array. The error isn't in the rendering code, but instead is here:
active_model_serializers-7b0a85fdda85/lib/active_model/serializer/array_serializer.rb:17:in initialize'`

    def initialize(objects, options = {})
        options.merge!(root: nil)

        @objects = objects.map do |object|
          serializer_class = options.fetch(
            :serializer,
            ActiveModel::Serializer.serializer_for(object)
          )
          serializer_class.new(object, options.except(:serializer))
        end
        @meta     = options[:meta]
        @meta_key = options[:meta_key]
      end

So it's the array serializer that needs to know how to handle non-serializer data.

@kayakyakr
Copy link
Author

Easier test to write. Much harder thing to fix because array serializer feeds that straight in to the adapter which doesn't know how to handle a non-serializer object or string.

@bf4
Copy link
Member

bf4 commented Jun 12, 2015 via email

@joaomdmoura
Copy link
Member

Hey ppl, just read trough the thread, I'm happy to see that you narrowed that the issue!
Just keep in mind that we are kind starting the fix the has_many at #955
I know that it's probably not related to array_serializer it self but just keep it in mind! 😄

@kayakyakr
Copy link
Author

That should fix it @joaomdmoura, or at least indicate how this should be fixed. It's the same issue at the core if it isn't the exact same issue.

@GriffinHeart
Copy link
Contributor

I think this is the same as #772

@kayakyakr
Copy link
Author

Yes, it is. Also the same as #955 and #877, or at least should be fixed by the fix for that issue.

@GriffinHeart
Copy link
Contributor

Just to add some info the nil comes from here

https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model/serializer.rb#L243-L247

def self.get_serializer_for(klass)
  serializers_cache.fetch_or_store(klass) do
    serializer_class_name = "#{klass.name}Serializer"
    serializer_class = serializer_class_name.safe_constantize

    if serializer_class
      serializer_class
    elsif klass.superclass
      get_serializer_for(klass.superclass)
    end
  end
end

When using render json: ['errors'] class will be String which has no serialiser, same thing as render json: [{a: 1}] where class will be Hash which also has no serialiser, thus serializer_class is nil

@joaomdmoura
Copy link
Member

It was fixed at #962
I'm closing this for now, but would be awesome if you guys could test it yourself 😄

@kayakyakr
Copy link
Author

Seems to be working. Had to switch up to using the FlattenJson serializer for some of the parts where I was building up a serializable hash outside of the context of the controller, but that was an unrelated change.

@phuongnd08
Copy link

This issue has been re-introduced in 0.10.5. "render [{}]" is returning error.

UncaughtThrowError: uncaught throw :no_serializer

@bf4
Copy link
Member

bf4 commented Mar 22, 2017

@phuongnd08 You've commented on an issue from over a year ago without sufficient information to debug. I can't think of a realistic scenario where you'd be using AMS and render an array with a hash in it to json. Please open a new issue.

@rails-api rails-api locked and limited conversation to collaborators Mar 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants