Bug #15977

Unobvious result value from #next_rotate_time and #previous_period_end methods of Logger::Period module

Added by davydov_anton (Anton Davydov) over 1 year ago. Updated about 1 year ago.

Target version:
ruby -v:
2.5 - master


I found that #next_rotate_time and #previous_period_end methods can return different time objects in some case.

When we call these methods with valid shift_age option methods returns a new Time object with the local machine's timezone. If you put invalid shift_age argument it will return existed time object with any timezone. Script for better understanding and reproducing:

require 'time'
require 'logger'

time = Time.parse("2019-07-18 13:52:02 -0300")

result = Logger::Period.next_rotate_time(time, 'daily')
invalid_arg_result = Logger::Period.next_rotate_time(time, 'invalid')

p ==

In this case, result object will have my local timezone and invalid_arg_result will have -03 timezone.

I think that the problem in these lines:

The question is: is it good and if not what the way we can use to fix it?

Updated by mame (Yusuke Endoh) over 1 year ago

  • Assignee set to sonots (Naotoshi Seo)
  • Status changed from Open to Assigned

Updated by sonots (Naotoshi Seo) about 1 year ago

  • Description updated (diff)

Updated by sonots (Naotoshi Seo) about 1 year ago

I feel raising an ArgumentError for invalid value is the way to fix. It breaks backward behavior, but it is probably acceptable and a correct way to handle an invalid value.

Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

I've submitted a pull request that raises ArgumentError for invalid shift_age values:

For backwards compatibility and to keep the tests passing, values of 'now' and 'everytime' are considered valid and result in the behavior of rotating every time.


Updated by jeremyevans0 (Jeremy Evans) about 1 year ago

  • Status changed from Assigned to Closed

Also available in: Atom PDF