introduce Date::InvalidDateError, DateTime::InvalidDateTimeError

[glaszig]

all too often i see and write the following:

def foobar value
  Date.parse value
rescue ArgumentError => e
  if e.message == "invalid date"
    # handle that invalid date
  else
    raise
  end
end

with this patch this code will still work (because Date::InvalidDateError < ArgumentError) but can become this:

def foobar value
  Date.parse value
rescue Date::InvalidDateError
  # handle that invalid date
end

i think ArgumentError is far too generic for the case of failing because of an invalid date and we are dealing with dates quite often. that's why i want to introduce a more specific error type to make it easier to target that case in code bases and not running the risk of rescuing unrelated ArgumentError's with rescue ArgumentError when actually just wanting to deal with an invalid date that we get from user-input.

there also is a DateTime::InvalidDateTimeError class which works exactly the same:

def foobar value
  DateTime.parse value
rescue DateTime::InvalidDateTimeError
  # handle that invalid date
end

what do you think?

[glaszig]

just realized we‘re gonna need the same for DateTime. will add that later.

[glaszig]

renamed Date::DateError to Date::InvalidDateError. added an analogous DateTime::InvalidDateTimeError. i think those are better names and having one per "module" is the better way.

[ioquatix]

How about ParseError

[ioquatix]

I thought a bit more about this.

You probably want to introduce the following:

class Date::Error < StandardError
end

class Date::ParseError < Date::Error
end

class DateTime::ParseError < DateTime::ParseError
end

There are probably other ways to structure it but the key point is:

• You should be able to catch any date/time related error without catching general errors.
• You should be able to select parse errors out of that, or at least they should be specialised because they relate to a specific kind of circumstance.

[glaszig]

that’s fine with me. what about compatibility, code that expects ArgumentError?

[ioquatix]

Maybe it should just be ParseError < ArgumentError?

[glaszig]

fine with me as well. makes more sense, too. done.

[jeremyevans]

This appears to use Date::ParseError/DateTime::ParseError in cases where there is no parsing, which I think is confusing. Date::Error or Date::InvalidDateError both seem like better names. I don't think an error specific to date parsing is worth adding.

I would actually just add Date::Error < ArgumentError. DateTime::Error would refer to it as DateTime subclasses Date. I don't think there is a need to separate Date errors from DateTime errors, and if there is a need in the future, it is easy to make DateTime::Error < Date::Error.

[glaszig]

I would actually just add Date::Error < ArgumentError. DateTime::Error would refer to it as DateTime subclasses Date

done. do you see other raises that need to be adjusted? i'm not exactly sure.

[jeremyevans]

I think every place that raises ArgumentError with invalid * messages should probably be converted to Date::Error. There appears to be some cases that still need to be updated.

[glaszig]

I think every place that raises ArgumentError with invalid * messages should probably be converted to Date::Error. There appears to be some cases that still need to be updated.

done. rebase + fixup?

[jeremyevans]

This looks good to me. Assuming no objections, I plan to merge it before the release of Ruby 2.7.

[glaszig]

thanks. i‘ll rebase + fixup this weekend.