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

Fix empty template source #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fnordfish
Copy link

When rails compiles a template, it nils-out it's source.
For most (normal) requests this does not matter, but when running a bunch of tests this may happen (at least to me)

When the template got compiled by rails the source is `nil`
@ccocchi
Copy link
Owner

ccocchi commented Jan 6, 2013

When do you run into a empty source in a compiled template ?

@ccocchi
Copy link
Owner

ccocchi commented Jan 17, 2013

@fnordfish any news?

@fnordfish
Copy link
Author

Hi @ccocchi, thanks for reminding me...

Well, we run a lot of rspec tests against the JSON-API our Rails app exposes.
When we run the hole testsuite (or a larger context) the content of some partials (or "extends") is missing. When we run those failing tests separately, it's all fine.

Do you think this is more a Rails or RSpec issue?

@ccocchi
Copy link
Owner

ccocchi commented Jan 17, 2013

No I think it is a rabl-rails weird behavior.

Did you try to set cache_templates to false in your test environment ?

@fnordfish
Copy link
Author

Yes, I have tried to set cache_templates to false, doesn't make any difference.
I tracked it down to ActionView::Template#compile! where the source value of the template gets "deleted". Somehow this Template instance gets recycled, and I found no proper way of getting the "raw source" again.

@ccocchi
Copy link
Owner

ccocchi commented Jan 18, 2013

I use RSpec for testing 2 fairly large applications using rabl-rails and I never ran into this problem.

But I dont really see why Rails nil the source in this case, since @compiled will stay true. I'll try to check this issue but if you can reproduce a small app with the issue that would be fantastic.

@fnordfish
Copy link
Author

I'll try to build one. My guess is, that this happens to "partial templates" - but I'm not really sure. We converted our rabl app to rabl-railsm (which was fairly straight forward btw) so there was no last change to check.
Maybe @Tonkpils can dis/approve my guess, his error really looks so alike.

@Tonkpils
Copy link

So it seems @fnordfish commit fixes the issue.
@ccocchi I ran my rspec suite in different ways.

WIth Rabl-Rails without this commit
Using just rspec spec no seed and I received about 6 different failures all the same error message as issue #16.
Using rspec spec --seed 43614 which was a known seed to only yield 4 failures and got same expected 4 failures.
Also, I ran the suite with focus on smaller sets of tests. I got to the point to only focusing on the tests that yield the 4 failures and they passed focusing on just those 4. Once I ran the entire file where the failures were present the failures came back.

With this commit
I ran the same tests, seems so far that the errors are not back. I ran them under the same conditions and made sure to tests random seeds plus the seed that yielded the same errors.

So +1 from me on this PR. 👍

@fnordfish
Copy link
Author

So, I've created an test app and in this one I cannot reproduce this.

However, in my big-app I have partials and extends with /path/to/view. When I change some of them to path/to/view those will crash.
Actually, my first work around was to add those starting slashes, but after some time the same errors just popped up again.

@Tonkpils
Copy link

Pretty similar results with small tests sets. Like I said, if you have the rspec configuration to "random" it'll run the tests in different order and maybe its the same issue I was running into. When I used the --tag focus to only test a small set of the suit, the test passed with flying colors until I added more and more tests. My guess is that once rails renders certain amount of views, it nils the template source.

I switched to rabl-rails from @fnordfish fork on my big-app and I'm still getting errors but now at least they are different.

Api::V1::WebApiController applications #application_details with valid account id and app id includes the application name
     Failure/Error: post :application_details, account_id: account_id, application_id: client_app_id
     ActionView::Template::Error:
       Called id for nil, which would mistakenly be 4 -- if you really wanted the id of nil, use object_id
     # ./app/views/client_applications/details.json.rabl:2:in `_app_views_client_applications_details_json_rabl___2241378183412652733_70127943037700'
     # ./app/controllers/api/v1/web_api_controller.rb:262:in `application_details'
     # ./spec/controllers/api/v1/web_api_controller_spec.rb:103:in `do_app_details'
     # ./spec/controllers/api/v1/web_api_controller_spec.rb:110:in `block (5 levels) in <top (required)>'

Pretty much same issue with random errors popping up with different seeds but at least is a different issue. Not the nil source.

I'll debug and let you know what I find out.

@fnordfish
Copy link
Author

@Tonkpils seems, that your details.json.rabl is missing a object :@post

@ccocchi
Copy link
Owner

ccocchi commented Jan 18, 2013

Using File.binread is not the good solution to the problem, more a monkey fix. The problem is why ActionView is niling sometimes the source and sometimes not (my case).

@fnordfish actually I think i never used a starting slash in any of my extends/partial, I will test with it.

Anyway thanks to both of you.

@Tonkpils
Copy link

@fnordfish Yeah, I realized that as soon as I left the office. Agreed @ccocchi thanks for the awesome gem!

@lloydmeta
Copy link
Contributor

Hmm, running into this problem today. Not entirely sure why either.

@lloydmeta
Copy link
Contributor

In my case, setting cache_templates to false seems to have helped.

Spoke too soon: failed on another run.

I seem to have narrowed this down to the following scenario:

I had two controllers that were rendering to the same template, one by directly rendering the template and another by extending through a child:

object :@user

attributes  :id, :nickname

I changed the controller that was rendering the above template directly to just render to json without using the Rabl template and the problem went away.

Also interesting: the problem only occurred when the spec for controller that was rendering the above template directly was run BEFORE the spec for the template that rendered the above template through a child extension.

@freemanoid
Copy link

I can confirm it in my small-test app where I try to use rails-api. It appear SOMETIMES when I try to get "localhost:3000/tours.json". Rails server restart helps.
I will get an error:

ActionView::Template::Error (can't convert nil into String):
    1: collection :@tours
    2: extends "tours/show"
  app/views/tours/index.json.rabl:2:in `_app_views_tours_index_json_rabl___2066067952812563330_14493560'
  app/controllers/tours_controller.rb:6:in `index'

my repo and tag where this issue appears
https://github.com/ouyoutouchmytralala/travel-api/tree/strange-rails-api-issue

Additional information how to reproduce the issue:

  1. open url with show action like "tours/5110e8b5a8fefe71e0000197.json "
  2. try to open index action "tours.json" and you will see an error.

I find a workaround for it that works for me:
aa43c318f90ec60219e5f66b9add0dabccdafade
But I think that it is at least unexpected behavior, but does not feature of rabl-rails. And its not good.

@ccocchi
Copy link
Owner

ccocchi commented Mar 4, 2013

I cloned your repo and tried to reproduce your bug, but still no errors on my side :( With ruby 1.9.3, rails 3.2.11

@beedub
Copy link

beedub commented Mar 20, 2013

I had this same issue with partials, and isolated it to LookupContext improperly resolving paths without extensions (in a single session, it works, but stops). The seemingly stable fix was to use the full filename and extension rather than just name (eg. users/index.json.rabl vs users/index).

@Tonkpils
Copy link

It does seem adding extensions fixes the issue of source being set to nil. Digging a bit deeper into this.

Doing something like this seems to cause the issue.

shared/success.json.rabl

object false

node(:status) { :success }
node(:message) { @message }

cool/show.json.rabl

object false

node(:cool) { partial('cool/_show', object: @cool) }
extends 'shared/success'

Seems without an extension on shared/success, @lookup_context.find_template returns a template with nil source causing:

ActionView::Template::Error:
  no implicit conversion of nil into String

I'm not sure what would be the correct way of fixing this issue. If the only option is to make sure that the extends contains the proper extension , the documentation should reflect this. Thoughts @ccocchi

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

Successfully merging this pull request may close these issues.

None yet

6 participants