Skip to content
New issue

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

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

Already on GitHub? # to your account

add fallible API for TimeZone::system() #65

Closed
BurntSushi opened this issue Jul 31, 2024 · 1 comment
Closed

add fallible API for TimeZone::system() #65

BurntSushi opened this issue Jul 31, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@BurntSushi
Copy link
Owner

The jiff::tz::TimeZone::system() call is infallible, even though it can of course fail to find a system's configured time zone. If it fails in this regard, it will fall back to UTC. (As libc does, for example, if /etc/localtime doesn't exist on my Linux system.)

But, it would be nice if we had a TimeZone::try_system() that returned an error instead of an automatic fallback to UTC. This came up in GitoxideLabs/gitoxide#1474, where it was suggested that it would be nice to alert the end user of the system time zone couldn't be found.

One complication here is the error message. Currently, a lot of the "details" are written as trace! logs. And smushing all of those details into a single error message is probably the wrong way to go? On the other hand, just returning "couldn't find system configured time zone" is also probably not great either. So some thought should go into what the failure mode actually looks like here.

@BurntSushi BurntSushi added the enhancement New feature or request label Jul 31, 2024
BurntSushi added a commit that referenced this issue Aug 31, 2024
This API is like `TimeZone::system`, but turns "get system configured
time zone" into a fallible operation instead of automatically falling
back to UTC.

The error message here is pretty generic and uninformative. All of the
interesting bits are emitting as log messages. In #65, I talked about
improving this, but it's not quite clear the best way to do that. I
think we'd probably need to invent some smarter infrastructure for
collecting a "trace" of what was attempted, and then either emit them as
log messages or tie them up into a single error message. But the latter
case could result in a very big error message.

So for now, this just returns a pretty generic error message, but all of
the log statements are still there for when you need to understand more
deeply what went wrong.
BurntSushi added a commit that referenced this issue Aug 31, 2024
This API is like `TimeZone::system`, but turns "get system configured
time zone" into a fallible operation instead of automatically falling
back to UTC.

The error message here is pretty generic and uninformative. All of the
interesting bits are emitting as log messages. In #65, I talked about
improving this, but it's not quite clear the best way to do that. I
think we'd probably need to invent some smarter infrastructure for
collecting a "trace" of what was attempted, and then either emit them as
log messages or tie them up into a single error message. But the latter
case could result in a very big error message.

So for now, this just returns a pretty generic error message, but all of
the log statements are still there for when you need to understand more
deeply what went wrong.
@BurntSushi
Copy link
Owner Author

Oops, forgot to mark this as closed by #123.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant