Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

does ZonedDateTime.from with offset: reject always throw when the datetime is in a DST transition gap? if so, is that intended? #2892

Open
BurntSushi opened this issue Jun 8, 2024 · 3 comments
Labels

Comments

@BurntSushi
Copy link

(This is another issue where I'm asking a question to help aide my own understanding.)

Given this code snippet:

let t = Temporal.ZonedDateTime.from(
  "2024-03-10T02:30-04:00[America/New_York]",
  { offset: 'reject' },
);
console.log(t.toString());

I get this exception:

RangeError: Offset -04:00 is invalid for 2024-03-10T02:30:00 in time zone

Similarly, for this code snippet (the only change from above is the offset):

let t = Temporal.ZonedDateTime.from(
  "2024-03-10T02:30-05:00[America/New_York]",
  { offset: 'reject' },
);
console.log(t.toString());

I get this exception:

RangeError: Offset -05:00 is invalid for 2024-03-10T02:30:00 in time zone

Walking through this, I think the reason why these both throw is that the offset in the input wouldn't match the offset in the ZonedDateTime returned. Namely, I believe that 2024-03-10T02:30-04:00[America/New_York] unambiguously refers to 2024-03-10T06:30Z and 2024-03-10T02:30-05:00[America/New_York] unambiguously refers to 2024-03-10T07:30Z. In the former case, the correct zoned instant is actually 2024-03-10T01:30-05:00[America/New_York], and in the latter case, it's 2024-03-10T03:30-04:00[America/New_York]. As you can see, the offset flip flops. I think that is true for every gap. So I think that in turn means that offset: reject always throws for any datetime in a gap, even if it has a correct offset that disambiguates it.

I guess my question here is, is this behavior intended? And if so, what is the rationale behind it? That is, is it preventing the caller from making a mistake somehow? My understanding is that if the offset makes the datetime unambiguous (as I believe it does in this case), then the offset is "valid" and maybe shouldn't result in an error. On the other hand, if offset: reject is just about whether the offset in the input matches the offset in the output precisely, then I think the exception is warranted. I'm just having trouble understanding what would go wrong if the datetimes above were parsed without error.

@ptomato
Copy link
Collaborator

ptomato commented Sep 9, 2024

Apologies for not getting to this earlier.

This is my understanding of the rationale. One of the reasons offset: "reject" exists is for flushing out strings that were valid times when they were serialized, but have become invalid due to political time zone changes. So if ...T02:30-04:00 or ...T02:30-05:00 were not in a DST gap when you serialized them, the exception would signal to you that they now were, and that your wall clock time has changed for that instant.

@justingrant may have a better understanding than me.

@justingrant
Copy link
Collaborator

One of the reasons offset: "reject" exists is for flushing out strings that were valid times when they were serialized, but have become invalid due to political time zone changes.

Sorry for delayed response, I just got back from a long work trip.

@ptomato is correct. The purpose of offset: "reject" is to detect cases where once-valid strings are now invalid. In order to support this case, we apparently lost the ability to use offset: "reject" to parse times in a DST gap. I'm not 100% sure this is the best tradeoff, but like many things related to DST it's a judgement call, and IMO it's not necessarily the wrong call either for offset: "reject" to be as restrictive as possible.

At this point in the proposal where we're very close to Stage 4, I think the bar is quite high to consider this a spec bug. We'd need to have another reason to consider a normative change to this behavior.

@justingrant
Copy link
Collaborator

One more thing: because the requested change is making an exception case into a non-throwing case, if the current behavior proves to be wrong then it can likely be changed in a follow-up proposal without breaking apps. Which is another reason that we probably shouldn't change this behavior now! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants