Project

General

Profile

Actions

Bug #15977

closed

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

Added by davydov_anton (Anton Davydov) over 5 years ago. Updated about 5 years ago.

Status:
Closed
Target version:
-
ruby -v:
2.5 - master
[ruby-core:93491]

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)
Actions #2

Updated by sonots (Naotoshi Seo) over 5 years ago

  • Description updated (diff)

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.

Actions #5

Updated by jeremyevans0 (Jeremy Evans) about 5 years ago

  • Status changed from Assigned to Closed
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0