time: LoadLocation returns undescriptive error messages if no time zone database is available #50248

GreenLightning · 2021-12-17T19:47:13Z

What version of Go are you using (`go version`)?

go version go1.17 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system are you using?

Windows

What did you do?

Attempt to load time zone information using time.LoadLocation() and test under various conditions where no time zone database is available.

This issue is made worse by the fact that the Go distribution includes a time zone database, but it is not embedded into programs being compiled by default. Therefore binaries of my program worked fine for me, but not for my users that did not have Go installed. The "works on my machine" syndrome combined with the very generic error message made identifying the root issue a lot more difficult than it should have been.

Therefore to reproduce this issue, you have to be on Windows (where no system-wide time zone database is available) and temporarily rename the folder $GOROOT/lib/time to something else.

[If you are only here because your calls to LoadLocation() return the same error, the solution to this problem is to import _ time/tzdata to embed a fallback copy of the database into the binary. See the time/tzdata package documentation for more details.]

package main

import (
	"fmt"
	"time"
)

func main() {
	_, err := time.LoadLocation("EST")
	fmt.Println("Error:", err)
}

What did you expect to see?

A clear error message that no time zone database is available.

What did you see instead?

Error: The system cannot find the path specified.

This error is very generic. Compare for example with os.Open(), which returns an error of the form open <filename>: The system cannot find the path specified.. Even better would be a specific error message for the case that none of the searched locations contained a time zone database.

Interestingly, if you rename the zip file inside $GOROOT/lib/time instead of the directory itself then you get a different nonsensical error Error: unknown time zone EST, because ENOENT is ignored here. I think separate cases are confused in this logic. In the case where the time zone database is a plain directory, ENOENT means that there is no file for this specific time zone, and ignoring the error is correct. However, ENOENT can also be returned from loadTzinfoFromZip if the zip file cannot be found, in which case I would expect the error to be returned further. The same problem can be found here regarding the ZONEINFO environment variable.

The text was updated successfully, but these errors were encountered:

rlanhellas · 2022-01-14T22:11:27Z

Hi @GreenLightning, I agree with you, the error in windows is difficult to understand without knowledge of how Goland works with Timezone, the problem here is that Windows return different errors in different situations:

When the directory doesn't exist
syscall.ENOTDIR
When the file doesn't exist
syscall.ENOENT

But in Unix systems the return is always syscall.ENOENT. So, this line below, in loadLocation method, is just prepared for Unix systems:

if firstErr == nil && err != syscall.ENOENT {
			firstErr = err
		}

I did a test in my MacOS BigSur and Windows 10, with the line below working as expected in both systems:

if firstErr == nil && err != syscall.ENOENT && err != syscall.ENOTDIR {
			firstErr = err
		}

@ianlancetaylor , what do you think? Can I open a Merge Request with this fix and improve this error message?

ianlancetaylor · 2022-01-14T23:10:57Z

If we make such a change it should probably be done in time/sys_windows.go. It seems reasonable to do it there.

rlanhellas · 2022-01-15T01:53:32Z

I can't understand how changing time/sys_windows.go seems more reasonable, because methods like open call directly syscall.Open and the operational system returns ENOTDIR (3), there isn't a transformation or something else in sys_windows.go.

GreenLightning · 2022-01-15T14:47:39Z

My issue actually goes further than that.

1. No clear attribution of the error to the time package

Again, "The system cannot find the path specified." is a super generic error in Windows. My suggestion would be to wrap errors that are returned to the caller, e.g. like load location EST: The system cannot find the path specified. or similar. Compare with how os.Open() returns open example.txt: The system cannot find the path specified..

2. Behavior when no database is available

As mentioned in the original description ENOENT can mean two different things: a) the time zone database source is invalid, b) the source is valid, but does not contain an entry for this particular time zone.

I think ignoring the error in case b) is uncontroversial. Ignoring the error in case a) makes sense in the usual case (searching multiple common locations until a valid source is found). However, when all possible locations fail with case a) then I think a different error message should be returned, explicitly stating that no time zone database could be found instead of unknown time zone EST, which does not make any sense, because EST is a well-known time zone (or the error message should be changed, e.g. failed to load time zone EST).

Another case is when using ZONEINFO, if you misspell the value of ZONEINFO it will be silently ignored. Is this the intended behavior?

rlanhellas · 2022-01-15T15:56:51Z

My issue actually goes further than that.

1. No clear attribution of the error to the time package

Again, "The system cannot find the path specified." is a super generic error in Windows. My suggestion would be to wrap errors that are returned to the caller, e.g. like load location EST: The system cannot find the path specified. or similar. Compare with how os.Open() returns open example.txt: The system cannot find the path specified..

2. Behavior when no database is available

As mentioned in the original description ENOENT can mean two different things: a) the time zone database source is invalid, b) the source is valid, but does not contain an entry for this particular time zone.

I think ignoring the error in case b) is uncontroversial. Ignoring the error in case a) makes sense in the usual case (searching multiple common locations until a valid source is found). However, when all possible locations fail with case a) then I think a different error message should be returned, explicitly stating that no time zone database could be found instead of unknown time zone EST, which does not make any sense, because EST is a well-known time zone (or the error message should be changed, e.g. failed to load time zone EST).

Another case is when using ZONEINFO, if you misspell the value of ZONEINFO it will be silently ignored. Is this the intended behavior?

All the discussion is about improving error messages to help the developer solve problems in the fastest way. I really don't think misspelling ZONEINFO env should return an error because it can be another env variable and not a misspelling, so it's correct, we can't consider an error.

About improving messages to tell exactly error and not just a The system cannot find the path ..., I think we can change zoneinfo.go and zoneinfo_read.go to improve these messages, but I'm waiting to @ianlancetaylor explain the reason to make this change in time/sys_windows.go instead.

ianlancetaylor · 2022-01-16T00:34:17Z

@rlanhellas In the time package the files sys_unix.go and sys_windows.go are intended to hide the differences between Unix and Windows. Checking for syscall.ENOTDIR is only required on Windows. Therefore, I think we should hide that difference by changing the open function in sys_windows.go to check for syscall.ENOTDIR (or actually it would likely be more clear to check for syscall.ERROR_PATH_NOT_FOUND) and in that case to return syscall.ENOENT.

rlanhellas · 2022-01-17T11:13:02Z

I see, because in this case, we just need to change in exactly one point and everything that is using syscall.ENOTDIR keeps working normally.

I will suggest a Merge Request with this changing, ok @ianlancetaylor ?

ianlancetaylor · 2022-01-17T21:34:33Z

Sure, thanks, though likely won't be submitted until after the 1.18 release.

rlanhellas · 2022-01-29T14:36:03Z

Hello @ianlancetaylor , I made git clone and created my branch following this guide: https://go.dev/doc/contribute#sending_a_change_github. But when I try to send the push I got permission denied:

11:33:25.498: [go-github] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false commit -F /private/var/folders/cq/n7ybvyp94_bfdsb_vx6984sw0000gn/T/git-commit-msg-.txt --
[fix-filenotfound-windows-timezonedb 9fee179f11] time: return ENOENT instead of ERROR_PATH_NOT_FOUND in windows
 1 file changed, 3 insertions(+)
11:33:27.381: [go-github] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false push --progress --porcelain origin refs/heads/fix-filenotfound-windows-timezonedb:refs/heads/fix-filenotfound-windows-timezonedb --set-upstream
ERROR: Permission to golang/go.git denied to rlanhellas.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.

Any tips?

seankhliao · 2022-01-29T14:58:41Z

@rlanhellas Changes via github are made by using forks and PRs, not pushing directly to the this (read only) repo. See github docs for more information.

When using windows some users got a weird error (File not found) when the timezone database is not found. It happens because some methods in the time package don't treat ERROR_PATH_NOT_FOUND and ENOTDIR. To solve it was added a conversion to ENOTENT error. Fixes golang#50248

rlanhellas · 2022-01-29T16:42:09Z

Thanks @seankhliao . @ianlancetaylor PR was opened: #50906.

gopherbot · 2022-01-29T16:45:51Z

Change https://golang.org/cl/381957 mentions this issue: time: return ENOENT instead of ERROR_PATH_NOT_FOUND in windows

When using windows some users got a weird error (File not found) when the timezone database is not found. It happens because some methods in the time package don't treat ERROR_PATH_NOT_FOUND and ENOTDIR. To solve it was added a conversion to ENOTENT error. Fixes golang#50248

ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 17, 2021

ianlancetaylor added this to the Backlog milestone Dec 17, 2021

rlanhellas mentioned this issue Jan 29, 2022

time: return ENOENT instead of ERROR_PATH_NOT_FOUND in windows #50906

Closed

jmspradlin mentioned this issue Apr 20, 2022

"The system cannot find the path specified." on time_zone check in pagerduty_schedule PagerDuty/terraform-provider-pagerduty#484

Closed

gopherbot closed this as completed in 4c565a5 May 8, 2022

golang locked and limited conversation to collaborators May 8, 2023

gopherbot added the FrozenDueToAge label May 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

time: LoadLocation returns undescriptive error messages if no time zone database is available #50248

time: LoadLocation returns undescriptive error messages if no time zone database is available #50248

GreenLightning commented Dec 17, 2021

rlanhellas commented Jan 14, 2022 •

edited

ianlancetaylor commented Jan 14, 2022

rlanhellas commented Jan 15, 2022 •

edited

GreenLightning commented Jan 15, 2022

rlanhellas commented Jan 15, 2022

1. No clear attribution of the error to the time package

2. Behavior when no database is available

ianlancetaylor commented Jan 16, 2022

rlanhellas commented Jan 17, 2022

ianlancetaylor commented Jan 17, 2022

rlanhellas commented Jan 29, 2022

seankhliao commented Jan 29, 2022

rlanhellas commented Jan 29, 2022

gopherbot commented Jan 29, 2022

time: LoadLocation returns undescriptive error messages if no time zone database is available #50248

time: LoadLocation returns undescriptive error messages if no time zone database is available #50248

Comments

GreenLightning commented Dec 17, 2021

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system are you using?

What did you do?

What did you expect to see?

What did you see instead?

rlanhellas commented Jan 14, 2022 • edited

ianlancetaylor commented Jan 14, 2022

rlanhellas commented Jan 15, 2022 • edited

GreenLightning commented Jan 15, 2022

1. No clear attribution of the error to the time package

2. Behavior when no database is available

rlanhellas commented Jan 15, 2022

1. No clear attribution of the error to the time package

2. Behavior when no database is available

ianlancetaylor commented Jan 16, 2022

rlanhellas commented Jan 17, 2022

ianlancetaylor commented Jan 17, 2022

rlanhellas commented Jan 29, 2022

seankhliao commented Jan 29, 2022

rlanhellas commented Jan 29, 2022

gopherbot commented Jan 29, 2022

What version of Go are you using (`go version`)?

rlanhellas commented Jan 14, 2022 •

edited

rlanhellas commented Jan 15, 2022 •

edited