Bug #15977
closedUnobvious result value from #next_rotate_time and #previous_period_end methods of Logger::Period module
Description
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 result.zone == invalid_arg_result.zone
p result.zone
p invalid_arg_result.zone
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:
https://github.com/ruby/ruby/blob/655b3da03ca31576d2318e674d71ff52b58c887a/lib/logger.rb#L635
and
https://github.com/ruby/ruby/blob/655b3da03ca31576d2318e674d71ff52b58c887a/lib/logger.rb#L654
The question is: is it good and if not what the way we can use to fix it?
Updated by mame (Yusuke Endoh) over 5 years ago
- Status changed from Open to Assigned
- Assignee set to sonots (Naotoshi Seo)
Updated by sonots (Naotoshi Seo) over 5 years 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 5 years ago
I've submitted a pull request that raises ArgumentError for invalid shift_age values: https://github.com/ruby/logger/pull/42
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 5 years ago
- Status changed from Assigned to Closed