#5555 actionmailer 2.3.9 should now require 'uri' - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5555 ✓resolved

actionmailer 2.3.9 should now require 'uri'

Reported by Hans de Graaff | September 5th, 2010 @ 10:02 AM | in 2.3.10

The actionmailer 2.3.9 tests failed for me when running against actionpack 2.3.9:

/usr/bin/ruby18 -I"lib:test" "/usr/lib64/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb" "test/asset_host_test.rb" "test/delivery_method_test.rb" "test/mail_helper_test.rb" "test/mail_layout_test.rb" "test/mail_render_test.rb" "test/mail_service_test.rb" "test/quoting_test.rb" "test/test_helper_test.rb" "test/tmail_test.rb" "test/url_test.rb" 
/usr/lib64/ruby/gems/1.8/gems/activesupport-2.3.9/lib/active_support/dependencies.rb:466:in `load_missing_constant': uninitialized constant ActionController::UrlWriter::URI (NameError)
    from /usr/lib64/ruby/gems/1.8/gems/activesupport-2.3.9/lib/active_support/dependencies.rb:106:in `const_missing'
    from /usr/lib64/ruby/gems/1.8/gems/actionpack-2.3.9/lib/action_controller/url_rewriter.rb:96
    from /var/tmp/portage/dev-ruby/actionmailer-2.3.9/work/ruby18/actionmailer-2.3.9/lib/action_mailer/base.rb:250
    from ./test/abstract_unit.rb:21
    from /usr/lib64/ruby/site_ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require'
    from /usr/lib64/ruby/site_ruby/1.8/rubygems/custom_require.rb:31:in `require'
    from ./test/asset_host_test.rb:1
    from /usr/lib64/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `load'
    from /usr/lib64/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
    from /usr/lib64/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `each'
    from /usr/lib64/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
rake aborted!
Command failed with status (1): [/usr/bin/ruby18 -I"lib:test" "/usr/lib64/r...]

This happens because http://github.com/rails/rails/commit/f8f4872fccbc6ba2b4970e4e9eab9c... now adds a dependency on URI that is not accounted for in actionmailer. I've include a patch to fix this, but I'm not sure if this is where you'd want the require to be.

Comments and changes to this ticket

You flagged this item as spam.
ronin-116156 (at lighthouseapp) September 16th, 2010 @ 06:32 PM
Hi Hans. I don't see your patch attached to this ticket.
You flagged this item as spam.
Hans de Graaff September 20th, 2010 @ 07:18 PM
Oops. Patch attached.
- actionmailer-239-require-uri.patch 507 Bytes
David Trasbo September 21st, 2010 @ 06:32 AM
- State changed from “new” to “open”
- Importance changed from “” to “Low”
Hans,

Could you please attach a patch according to the guidelines outlined here?

https://rails.lighthouseapp.com/projects/8994/sending-patches

Thanks! :)
Ryan Bigg September 21st, 2010 @ 03:04 PM
@David: I think that if the tests aren't running then isn't them then running enough? What in the guidelines has he missed?

@OP: For additional points: Is it just rake test and it breaks from within actionmailer? Nothing else magical?
David Trasbo September 21st, 2010 @ 04:51 PM
Hans and Ryan,

Perhaps I could have been more clear. :)

What's missing is to format the patch using format-patch to include commit message, author, etc.
You flagged this item as spam.
Tom Stuart September 27th, 2010 @ 09:32 AM
This bug prevented our app from bootstrapping when we upgraded to 2.3.9, possibly because of load order, so it's not just a test issue. Can the priority be made higher, please?
David Trasbo September 27th, 2010 @ 03:01 PM
- Importance changed from “Low” to “Medium”
Done.

Hans, please attach a patch according to the guidelines or will. Consider yourself warned. :)
You flagged this item as spam.
Hans de Graaff September 27th, 2010 @ 06:00 PM
I won't be adding a patch according to the guidelines, because I'm not convinced that this is the right approach to fixing it, even if it fixes the symptom. I'd rather see the owners of those components decide what the best way to fix this is. I've included my original patch to indicate exactly where the symptom surfaces.
Ryan Bigg September 27th, 2010 @ 11:19 PM
Hans, the correct approach to fixing it would be to put this require at the top of actionmailer/lib/action_mailer/base.rb with the other requires. Please write a patch to do this so that it is attributed to you.
Geoff Buesing October 6th, 2010 @ 04:03 AM
I had opened a duplicate ticket for this -- #5760 -- and Ryan Bigg asked me to attach my patch to this one.

My patch adds require 'uri' to the top of action_controller/url_rewriter, which is where it ultimately belongs, because this is the file that depends on URI, not action_mailer/base.

I consider this a high priority fix -- the framework won't boot correctly if you configure ActionMailer in your environment. In my case, I was configuring ActionMailer only in my production environment, which meant I deployed code (after upgrading to Rails 2.3.9) that worked in development and test, but wouldn't boot in production.
- require_uri.diff 820 Bytes
Rohit Arondekar October 6th, 2010 @ 07:19 AM
- Milestone set to “2.3.10”
- Assigned user set to “José Valim”
- Importance changed from “Medium” to “High”
You flagged this item as spam.
Hans de Graaff October 6th, 2010 @ 07:21 AM
I agree that it is much better to fix this issue in ActionController instead.
Geoff Buesing October 7th, 2010 @ 08:39 PM
Ok, I inspected my app a little more and realized that I was doing something unconventional -- I was setting ActionMailer settings in config/environments/production.rb directly on the constant, like so:
```
ActionMailer::Base.smtp_settings = {
  # settings
}
```
instead of the correct way for environment files:
```
config.action_mailer.smtp_settings = {
  # settings
}
```
Additionally, I was opting out of ActiveResource, which requires 'uri':
```
config.active_resource -= [:active_resource]
```
This non-conventional config + ActiveResource opt-out is probably why this issue has not been discovered by more users.

So, not as high-priority an issue as I first thought, though this should still be fixed.
David Trasbo October 7th, 2010 @ 08:43 PM
- Importance changed from “High” to “Medium”
David Trasbo October 10th, 2010 @ 04:26 PM
- Assigned user changed from “José Valim” to “Santiago Pastorino”
Santiago Pastorino October 10th, 2010 @ 05:20 PM
- Assigned user changed from “Santiago Pastorino” to “Mikel Lindsaar”
David Trasbo October 10th, 2010 @ 05:58 PM
Santiago,

Same as #3431 - except it's a very minor change. I think you should just go ahead and apply this. If you have time?
José Valim October 10th, 2010 @ 06:02 PM
- Assigned user changed from “Mikel Lindsaar” to “José Valim”
Leave it assigned to me.
David Trasbo October 10th, 2010 @ 06:09 PM
José, same as #3431.
Repository October 11th, 2010 @ 11:58 PM
- State changed from “open” to “resolved”
(from [f2e32e4fd7332d61324daaf33e99fd7f2e8e4c2a]) require 'uri' in action_controller/url_rewriter [#5555 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/f2e32e4fd7332d61324daaf33e99fd...