RailsApps / rails-stripe-membership-saas

An example Rails 4.2 app with Stripe and the Payola gem for a membership or subscription site.
http://railsapps.github.io/rails-stripe-membership-saas
1.14k stars 232 forks source link

PreCompiling Assets strips setPublishableKey #29

Closed stevecastaneda closed 11 years ago

stevecastaneda commented 12 years ago

Not sure why just yet, but precompiling assets is stripping the value for Stripe.setPublishableKey in registrations.js.erb. So when launching on Heroku, I can't create any users and seed data fails after creating roles.

Uncaught Error: You did not set a valid publishable key.
Call Stripe.setPublishableKey() with your publishable key.
For more info, see https://stripe.com/docs/stripe.js js.stripe.com:1
e.validateKey js.stripe.com:1
e.createToken js.stripe.com:1
n.processCard application-d1b920f96b33378ed3abb4e2d99b825e.js:17
(anonymous function) application-d1b920f96b33378ed3abb4e2d99b825e.js:17
v.event.dispatch application-d1b920f96b33378ed3abb4e2d99b825e.js:15
o.handle.u

Reviewing application-d1b920f96b33378ed3abb4e2d99b825e.js (compiled JS), setPublishableKey is as follows: (excerpt)

.done(function(e,t){Stripe.setPublishableKey("");
stevecastaneda commented 12 years ago

Went through guide for deploying to Heroku again, and noticed I was missing:

# Heroku requires this to be false
    config.assets.initialize_on_precompile=false

Added that and now getting a precompile error:

rake aborted!
uninitialized constant STRIPE_PUBLIC_KEY

Researching...

Update: Have to comment out that line before precompiling because of the call to the global constant STRIPE_API_KEY being set in stripe.rb.

tmock12 commented 12 years ago

Have you pushed your keys up to heroku?

stevecastaneda commented 12 years ago

Yessir. Testing in console I can call them both. But reviewing the compiled assets, it would never be added, which is what is triggering that console error (found using Chrome inspector). I'm trying to figure out why it's doing that.

Now, I've been having issues with my local environment variables not being set, which come to think of it, is why precompiling is not including them. BUT - wouldn't including the precompiled keys be a security risk anyway? So that couldn't be the cause could it (local environment ENV not being set when precompiling)?

Further reading: http://stackoverflow.com/questions/9235292/how-can-i-run-some-initializers-when-doing-a-rails-assetsprecompile

Lead me to: https://devcenter.heroku.com/articles/labs-user-env-compile

tmock12 commented 12 years ago

If your precompiling the javascript files then yes you need to have your variables(specifically STRIPE_PUBLIC_KEY) working at the time of precompile. You aren't putting your secret key into the javascript file, you are only putting your publishable key in there.

Heroku compiles your assets for you so you shouldn't need to precompile locally anyways.

stevecastaneda commented 12 years ago

Ah, I see. I was thinking that both keys were sensitive. I'll fix that today. Thanks for the direction! I realize that Heroku can precompile for me, but was referencing this page (linked from the Readme) which suggests to precompile locally.

While runtime asset compilation will work, it should be used as a last resort. Using runtime compilation will require Rails to compile your assets each time a dyno boots up increasing the wait time for a new dyno to become available. If you do nothing and rely on Heroku to perform slug compilation (when you deploy to Heroku), you may find Heroku is unable to compile the assets and will default to runtime compilation. It’s best to precompile the assets before deployment to Heroku.

http://railsapps.github.com/rails-heroku-tutorial.html

tmock12 commented 12 years ago

Yes I could see where that would confuse someone. And your more than welcome to precompile locally, but would have to make sure the public key ends up in that javascript file. either by manually typing it in there or having it set locally before precompile.

Personally I prefer to let heroku do the precompiling for me (They have gotten really good at it), then if something goes wrong I will precompile locally and push as a second option. and then, like in the article, as a third option resort to runtime compilation. I have never had to use that 3rd step thankfully.

Glad I could help point you in the right direction and best of luck

DanielKehoe commented 12 years ago

Could you clarify for me, please? Is this issue a bug in the rails-stripe-membership-saas app or was it a result of a local issue?

As currently implemented, the rails-stripe-membership-saas app can be set to precompile assets locally and it will work, as long as the local environment variables are set correctly?

@stevecastaneda had a problem because his local environment variables were not configured properly?

stevecastaneda commented 12 years ago

That's correct @DanielKehoe . Looks like it was a local issue that wasn't including the publishable key in the precompiled assets, causing me to have a "token missing" error in production. The precompile locally vs. on Heroku confusion on my end came from reading the launch to Heroku guide, and with me assuming both keys from Stripe were sensitive and needed to be loaded from the environment variables at all times (rather, publishable is ok and actually needs to be included in the precompiled assets for Stripe to work from what I understand now).

Should I close?

DanielKehoe commented 12 years ago

Thanks for the cogent clarification. I'll close.

tulbox commented 11 years ago

I would argue that it is a "bug", or at least something that should be documented:

When deploying the rails-stripe-membership-saas to heroku, in order to run rake assets:precompile it needs to have access to the ENV['STRIPE_PUBLIC_KEY'] config variable. As per https://devcenter.heroku.com/articles/rails3x-asset-pipeline-cedar#the-asset-pipeline

The app’s config vars are not available in the environment during the slug compilation process. Because the app must be loaded to run the assets:precompile task, any initialization code that requires existence of config vars should gracefully handle the nil case.

As currently configured, this does not gracefully handle the nil case, nor should it because the config variable value needs to be a part of the compiled .js asset when it is pre-compiled (you need the public key there, not a nil value).

After experimenting with a couple alternatives, I ended up going with the option presented by Ryan Bates in http://railscasts.com/episodes/288-billing-with-stripe. In a nut shell, I removed the need for mixing the variable in registrations.js while still utilizing a config variable: it's available via a metatag which is called within registrations.js (no longer registrations.js.erb).

//registrations.js
Stripe.setPublishableKey($('meta[name="stripe-key"]').attr('content'))

//application.html.erb
   <%= tag :meta, :name => "stripe-key", :content => STRIPE_PUBLIC_KEY %>
   ...
 </head>

Now we happily get:

   -----> Preparing app for Rails asset pipeline
   Running: rake assets:precompile
   Asset precompilation completed (52.45s)

If anyone has another suggestion, I'd be more than happy to hear them.

tmock12 commented 11 years ago

While the meta thing looks cool and i'd definitely be interested in exploring it. I feel that adding the keys to heroku is pretty well documented:

You’ll need to set your Stripe API and public key as Heroku environment variables. See the article Rails Environment Variables for more information. Here’s how to set Heroku environment variables for your Stripe API and public key:
$ heroku config:add STRIPE_API_KEY=secret STRIPE_PUBLIC_KEY=secret

Does the meta tag still work without precompiled assets?

Would you be interested in submitting a pull request with the above changes and any appropriate tests? I'm sure Daniel would love to have them. Thanks for the suggestion!

DanielKehoe commented 11 years ago

So, if I understand this:

1) Currently, the only way to successfully deploy to Heroku is to precompile assets locally. 2) Adding the meta tag trick would enable a second option, allowing Heroku to compile assets on deploy.

I'd seen Ryan's meta tag trick previously and didn't see any value in it (other than it is clever). But it seems it would allow a Heroku deploy without precompiling assets locally.

Is this correct?

stevecastaneda commented 11 years ago

@DanielKehoe Looks like it. I'm going to try this out on my end as well, because I've been having to precompile locally and manually place the public key in the compiled assets because my local ENV's are screwed. (using RVM and after hours, still can't get .bashrc nor .profile to load local ENV variables).

@tulbox Did you remove the .erb extension from registrations.js because we're no longer mixing in Ruby? Or was there another reason?

Update: Just tested and precompiled on Heroku just fine. Would like to suggest this update to the core app if possible.

DanielKehoe commented 11 years ago

@stevecastaneda Thanks for the testing and evaluation. Regarding your continuing problems with local environment variables, take a look at the article http://railsapps.github.com/rails-environment-variables.html. @tmock12 came up with a smart way to set environment variables without the Unix shell.

tulbox commented 11 years ago

Wow, four responses already - impressive.

@tmock12 again,

The app’s config vars are not available in the environment during the slug compilation process. Because the app must be loaded to run the assets:precompile task, any initialization code that requires existence of config vars should gracefully handle the nil case.

Therefore, the issue is not whether the vars can be set (no problems there - in fact, we're still using config vars in my proposed solution), it's the fact that they're not available during assets:precompile (as the app would need to be spun up, and there's absolutely nothing you can do about making them available).

@DanielKehoe

  1. You actually can't explicitly call rake assets:precompile even locally as again, local config vars are not available. I realized this belatedly. Of course, under normal development locally you're never going to have your app precompiling unless you specifically run the app under env.production (therefore, typically wouldn't run into this until you deploy to a production or staging environment, or explicitly test production locally).
  2. Using the metadata tag to hold your public key value and referencing it in the registrations.js (now no longer mixing in the one Ruby variable) makes any environment that calls assets:precompile, not just heroku, happy.

There are other alternatives: setting an inline javascript variable in each page with a dynamic ruby variable, hard-coding it in registrations.js, hidden field, etc., but the metatag solution seems cleanest and/or most maintainable to me (and it's easy to have it configured with no muss or fuss between development and production environments). It also dovetails well with Rails convention as per the Rails CSRF metatag implementation:

Ted

tulbox commented 11 years ago

@stevecastaneda As per @DanielKehoe's suggestion, absolutely switch to local_env.yml. Dead easy and works well across environments. Still use config vars in production (i.e. on heroku), but just make sure the local_env.yml for local non-production environments is in .gitignore and you're golden.

As well, yes, no .erb extension because now no longer mixing in the one Ruby var. No other reason apart from the fact that for static resources, they should really stay static. Without the .erb you have to think first before you are tempted to start mixing in Ruby vars ;-)

And lastly, @tmock12, I'd be happy to do a pull request. I can't think of any additional tests since the existing integration tests would fail if my proposed solution didn't work, and as a relatively n00b rails developer (long-time .NET developer), I don't know if there is an easy and efficient way to test rake assets:precompile (open to suggestions).

Ted

tmock12 commented 11 years ago
rake assets:precompile

Has access to your local config variables. So when you run that locally, your key should get set. I just tried it again and confirmed.

I can't confirm for heroku but I would assume it works the same as running rake locally. I will have more time to look into it later today.

tulbox commented 11 years ago

@tmock12 Very interesting. rake assets:precompile failed for exactly the same reason when I ran locally. I'll test when I do the pull request. Having said that, even if it does work locally, it is definitely a documented issue with heroku apps I.e. even if rake assets:precompile works locally in certain configurations and not in others, it is a known documented issue with heroku as per https://devcenter.heroku.com/articles/rails3x-asset-pipeline-cedar#the-asset-pipeline which explicitly states:

The app’s config vars are not available in the environment during the slug compilation process. Because the app must be loaded to run the assets:precompile task, any initialization code that requires existence of config vars should gracefully handle the nil case.

FYI I'm running ruby-1.9.3-p286 and Rails 2.3.8

Ted

tulbox commented 11 years ago

@stevecastaneda while on the subject of using yaml to hold config values, you might be interested in https://github.com/charlotte-ruby/yettings

tmock12 commented 11 years ago

@tulbox I'm in no way disagreeing with you. Just would like to know the details of why it's not working before I assume something and make a fix.

tulbox commented 11 years ago

@tmock12 Interestingly enough rake assets:precompile did work locally when I tested against my fresh fork, so like you, I'd like to know why. I do know that I was working against ruby-1.9.3-p327 at the time instead of ruby-1.9.3-p286. Apart from that, I can't think of any other reason why it would have failed locally previously (yes, multiple other changes including some additional gems, but nothing touching assets and the error was quite explicit about the error coming from registrations.js.erb on that line).

[Scratches head]

Ted

tulbox commented 11 years ago

@DanielKehoe realised two things belatedly: didn't branch my fork before doing the changes and also didn't update the readme...

DanielKehoe commented 11 years ago

@tulbox No problem. I can copy the changes without a pull request as they are only two lines. But if you want to update the README, I'd be very grateful.

tulbox commented 11 years ago

There was also a commit for adding the following to the application.rb:

# Deals with heroku issue when rake assets:precompile when deploying
# You may have to set it to false if you receive the following error when deploying to heroku:
#   could not connect to server: Connection refused
#   Is the server running on host "127.0.0.1" and accepting
#   TCP/IP connections on port xxxx?
# See https://devcenter.heroku.com/articles/rails3x-asset-pipeline-cedar#troubleshooting for more information.
# config.assets.initialize_on_precompile = false

The actual config setting is still commented out because it is only needed for heroku deployments. It would need to be noted in the readme as well which I'll attempt to get to early next week.

tmock12 commented 11 years ago

Just confirmed. heroku rake assets:precompile does have access to config variables. check out http://infinite-taiga-1458.herokuapp.com/ to see it in action.

you just have to add your keys to heroku before you deploy

tulbox commented 11 years ago

@tmock12 Thanks for attempting to confirm. I greatly appreciate your feedback on this entire thread.

However, I'd argue that it is doing runtime compilation (which as I'm sure you know you want to avoid) which means assets will compile at runtime (everytime the app starts up), not at deployment. It still works, but definitely not optimal.

To make sure I wasn't sending all of us on a wild goose chase after all this time, I went to confirm it myself:

  1. $ rails new rails-stripe-membership-saas -m https://raw.github.com/RailsApps/rails-composer/master/composer.rb -T
  2. Running under specific gemset for ruby-1.9.3-p327
  3. Set gemfile to use pg for production, and sqlite3 for :development, :test
  4. Set config.assets.initialize_on_precompile = false in application.rb because of Devise/heroku
  5. $ heroku create
  6. $ heroku config:add STRIPE_API_KEY=secret STRIPE_PUBLIC_KEY=secret
  7. Double check that set with heroku config. Yep. It is.
  8. $ commit everything
  9. $ git push heroku master
  10. $ Boom.

heroku-fail-vars

It will still works, but assets are compiled in the asset pipeline at runtime. See http://glacial-shelf-8278.herokuapp.com/ for the above deployment that "failed", but still "works":

 <script src="/assets/application-d9177708b33bea7639679e01919db103.js" type="text/javascript"></script>

Meaning "glacial" is a very applicable name (at least at app/dyno startup). And with such a simple solution (switch the public key out into a metatag and change the caller) even though it just applies to heroku.

If you did exactly the same as above and didn't encounter the same notification that assets:precompile failed, then perhaps it's a ruby gem version issue? But regardless, it is an officially documented outcome with heroku, so even if it may work in certain scenarios, it is documented to fail in many others.

Ted

tmock12 commented 11 years ago

ahah!! That's the issue! config.assets.initialize_on_precompile = false is what is keeping your rails app from loading before precompile. So while mine and other peoples are loading. your's is not. I missed that you had set that before.

So problem solved.

As a side note, you adding in that config setting is probably a good thing. It makes deploys a bit faster.

Thanks so much for being so responsive here and for your meta fix. I'll do some tests with the meta fix but so far it looks like a great addition and will give people the ability to set the config setting like you did, allowing for faster deploys.

oh and no, no runtime compilation for me. It's off. so if they didn't precompile my app would blow up.

tulbox commented 11 years ago

@tmock12 I verified before that yes indeed the config vars were set (see step 7 above). Even used my actual test keys even though dummy values would have been fine.

$ heroku config
....other keys removed here because otherwise you could truthfully call me an idiot ;-)...
STRIPE_PUBLIC_KEY:            pk_k5Ddznn9MBD3iQINm11XXdOEGKIpL

As to config.assets.initialize_on_precompile = false, it's actually in the Devise documentation (which even scrolls by as part of the generator output when you rails new rails-stripe-membership-saas -m....) whereby Devise is prevented from hitting the database during assets:precompile. See Devise specifically at https://github.com/plataformatec/devise#heroku, which states Rails 3.1, but it's also the case for 3.2.x, and more importantly http://guides.rubyonrails.org/asset_pipeline.html#precompiling-assets

It's most clear in the bit that scrolls by in the Devise setup as it explains why, but I dug quickly around in the Devise source and can't locate it.

I'm actually quite surprised you were able to deploy to heroku without first running into that issue. Without setting it, you should get the following error:

devise-heroku

Again, back to our favourite heroku documentation page (https://devcenter.heroku.com/articles/rails3x-asset-pipeline-cedar#the-asset-pipeline):

The app’s config vars are not available in the environment during the slug compilation process. Because the app must be loaded to run the assets:precompile task, any initialization code that requires existence of config vars should gracefully handle the nil case.

...and further down in the troubleshooting section...

If you see something similar to the following:

could not connect to server: Connection refused Is the server running on host "127.0.0.1" and accepting TCP/IP connections on port xxxx? This means that your app is attempting to connect to the database as part of rake assets:precompile. Because the > > config vars are not present in the environment, we use a placeholder DATABASE_URL to satisfy Rails. The full > > > command run during slug compilation is:

env RAILS_ENV=production DATABASE_URL=scheme://user:pass@127.0.0.1/dbname bundle exec rake assets:precompile 2>&1 scheme will be replaced with an appropriate database adapter as detected from your Gemfile While precompiling assets, in Rails 3.1.1 and up, you can prevent initializing your application and connecting to the database by ensuring that the following line is in your config/application.rb:

config.assets.initialize_on_precompile = false If rake assets:precompile is still not working, you can debug this locally by configuring a nonexistent database in your local config/database.yml and attempting to run rake assets:precompile. Ideally you should be able to run this command without connecting to the database.

What version of ruby and rails are you using?

Ted

tulbox commented 11 years ago

@tmock12 I guessed it had to do with config.assets.initialize_on_precompile = false but I figured there was no way you were able to deploy without it as it's a requirement for heroku. How does yours even deploy without it? Heck, even the official assets pipeline document says (in bold no less):

For faster asset precompiles, you can partially load your application by setting config.assets.initialize_on_precompile to false in config/application.rb, though in that case templates cannot see application objects or methods. Heroku requires this to be false.

Ted

tmock12 commented 11 years ago

yea that's an old issue with rails, heroku and rake. rake would try and load the whole rails app before the precompile and sometimes things would break. so before they actually fixed the problem they instructed everyone to add that config setting. The issue has since been fixed and you can deploy perfectly fine without it and let rake load up the whole rails environment before precompiling the assets. which is what gave me and the others the access to our config variables.

your and Ryan Bate's meta setup is much more friendly and dynamic though as it gives people the options of loading rails or not.

tulbox commented 11 years ago

@tmock12 Okay. Still curious as to why I am still never able to deploy to heroku without that config setting set...

stevecastaneda commented 11 years ago

@tulbox I think the answer you're looking for can be found here (under troubleshooting):

The most common cause of failures in assets:precompile is an app that relies on having its environment present to boot. Your app’s config vars are not present in the environment during slug compilation, so you should take steps to handle the nil case for config vars (and add-on resources) in your initializers.

DanielKehoe commented 11 years ago

We're now using the figaro gem and a config/application.yml file to set environment variables. Take a look and let us know if the new approach resolves these issues. (Details in the README.)

stevecastaneda commented 11 years ago

Ah, thanks @DanielKehoe. Playing catch up today on all the new stuff over the last week or so and saw @tulbox question and thought it was still unsolved. I'll go through my app this afternoon and try out all the new goodies.

DanielKehoe commented 11 years ago

Thanks everyone for the excellent research, discussion and collaboration! Let me know if I missed anything with the new commit.

tulbox commented 11 years ago

@stevecastaneda Yep. Quoted multiple times above. Actually, I think @tmock12 must have been getting sick with how many times I mentioned it ;-)

To sum up, when you've got:

config.assets.initialize_on_precompile = false our documented

config vars are then unavailable and therefore the ruby var mixed into registration.js.erb blew things up when deploying to heroku. My only remaining question was how @tmock12 and others were able to deploy to heroku without config.assets.initialize_on_precompile = false, as I've always had to include (exception described somewhere above). Regardless, as it helps with a faster deployment anyways because it doesn't spin up the app when you deploy/in order to deploy, the issue is asked, answered, and solved.

Ted