bitwalker / bitwalker/timex

Edge case when parsing 24:00 datetime in latest version

Open
#662 1 comment 6 reactions 0 assignees View on GitHub
Dominant language
Elixir
Stars
1.8k
Forks
407
PR merge metrics
No merged PRs in 30d

Description

Hi everyone, during an upgrade of a project to elixir 1.11, we updated Timex to the latest version (3.7.5), which resulted in a bunch of tests failing. We have to handle datetimes in this format: `2017-05-24 2400`, which come from an external service we do not have control on.

This is a weird format, as the [ISO 8601 article on Wikipedia](https://en.wikipedia.org/wiki/ISO_8601#Times) states:

```
As of ISO 8601-1:2019 midnight may only be referred to as "00:00", corresponding to the beginning of a calendar day. Earlier versions of the standard allowed "24:00" corresponding to the end of a day, but this is explicitly disallowed by the 2019 revision.
```

However, Timex worked perfectly before the update to 3.7.5... so I made a bunch of tests, using Timex versions ranging from 3.6.4 to 3.7.5, and elixir 1.10/1.11. I've tried calling this function:

```elixir
Timex.parse("2017-05-20 2400", "{YYYY}-{0M}-{0D} {h24}{0m}") |> elem(1) |> Map.get(:__struct__)
```

which resulted in the following:

**Elixir 1.10**:
- 3.6.4 -> returns `NaiveDateTime`
- 3.7.0 -> raises because it returns `:invalid_time` or `bad_match`
- 3.7.1 -> raises because it returns `:invalid_time` or `bad_match`
- 3.7.2 -> returns `NaiveDateTime`
- 3.7.3 -> returns `NaiveDateTime`
- 3.7.5 -> raises because it returns `:invalid_time` or `bad_match`

**Elixir 1.11**:
- 3.6.4 -> returns `NaiveDateTime`
- 3.7.0 -> raises because it returns `:invalid_time` or `bad_match`
- 3.7.1 -> raises because it returns `:invalid_time` or `bad_match`
- 3.7.2 -> returns `NaiveDateTime`
- 3.7.3 -> returns `NaiveDateTime`
- 3.7.5 -> raises because it returns `:invalid_time` or `bad_match`

At first I thought it was a breaking change between 3.6 and 3.7, but after more digging it looks like a regression to me... I tried digging into the code but it is way too overwhelming at the moment, hopefully I'll be able to do more after some feedback. Thank you :)

**EDIT**: [found](https://github.com/bitwalker/timex/commit/7337691fba9088bafd0c7674193c66f8ec06b44f#diff-a9dc4a824a8b0490b067c22dd68e4744cec4d6200b2111bf370d1471a3fa34c9R216) the offending line: `Time.new` returns `:invalid_time`:

```elixir
iex(1)> Time.new(23, 0, 0, 0)
{:ok, ~T[23:00:00.000000]}
iex(2)> Time.new(24, 0, 0, 0)
{:error, :invalid_time}
```

which looks fine to me, and certainly more of an issue with `Time` itself than timex... what do you think?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.