Timestamps timezone in exception doesn't match #9772

Closed
marclipovsky opened this Issue Mar 18, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@marclipovsky

When a Time object has a set timezone (e.g. EST) and an exception is raised on that object, the timestamp in the exception message has UTC timezone instead of the set timezone.

irb(main):018:0> time = Time.now.in_time_zone('EST')
=> Mon, 18 Mar 2013 07:53:14 EST -05:00
irb(main):019:0> time.foo
NoMethodError: undefined method `foo' for 2013-03-18 07:53:14 UTC:Time
    from /app/vendor/bundle/ruby/1.9.1/gems/activesupport-3.2.11/lib/active_support/time_with_zone.rb:332:in `method_missing'
    from (irb):19
    from /app/vendor/bundle/ruby/1.9.1/gems/railties-3.2.11/lib/rails/commands/console.rb:47:in `start'
    from /app/vendor/bundle/ruby/1.9.1/gems/railties-3.2.11/lib/rails/commands/console.rb:8:in `start'
    from /app/vendor/bundle/ruby/1.9.1/gems/railties-3.2.11/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'
@marclipovsky

This comment has been minimized.

Show comment
Hide comment
@marclipovsky

marclipovsky Mar 18, 2013

Here is time in UTC

irb(main):021:0> time.in_time_zone('UTC')
=> Mon, 18 Mar 2013 12:53:14 UTC +00:00

Here is time in UTC

irb(main):021:0> time.in_time_zone('UTC')
=> Mon, 18 Mar 2013 12:53:14 UTC +00:00
@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Mar 18, 2013

Member

confirmed! This is because TimeWithZone has a method_missing hook that delegates to the wrapped time. We could:

  1. only delegate methods that time.responds_to?().
  2. rescue NoMethodError and raise it from the context of TimeWithZone

@pixeltrix I'd implement 1.) but I think there are API's that rely on method missing. Let me know what you think and I submit a PR.

Member

senny commented Mar 18, 2013

confirmed! This is because TimeWithZone has a method_missing hook that delegates to the wrapped time. We could:

  1. only delegate methods that time.responds_to?().
  2. rescue NoMethodError and raise it from the context of TimeWithZone

@pixeltrix I'd implement 1.) but I think there are API's that rely on method missing. Let me know what you think and I submit a PR.

@pixeltrix

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Mar 18, 2013

Member

@senny I think option 2 is the safest - can you think of any other error messages that might need re-raising (i.e. include self.inspect in their message)?

Member

pixeltrix commented Mar 18, 2013

@senny I think option 2 is the safest - can you think of any other error messages that might need re-raising (i.e. include self.inspect in their message)?

@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Mar 18, 2013

Member

@pixeltrix I'm on it (implementing 2.). I don't think other errors need re-raising as this problem only occurs in combination with method_missing which eventually raises NoMethodError. If other errors occur it should not switch context.

Member

senny commented Mar 18, 2013

@pixeltrix I'm on it (implementing 2.). I don't think other errors need re-raising as this problem only occurs in combination with method_missing which eventually raises NoMethodError. If other errors occur it should not switch context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment