Project

General

Profile

Actions

Bug #15977

closed

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

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 6 years ago. Updated about 6 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 6 years ago Actions #1 [ruby-core:93492]

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

Updated by sonots (Naotoshi Seo) about 6 years ago Actions #2

  • Description updated (diff)

Updated by sonots (Naotoshi Seo) about 6 years ago Actions #3 [ruby-core:94024]

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 6 years ago Actions #4 [ruby-core:95322]

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 6 years ago Actions #5

  • Status changed from Assigned to Closed
Actions

Also available in: PDF Atom