SNOW-614539: incorrect error messages in writeOCSPCacheFile in ocsp.go

[setharnold]

gosnowflake/ocsp.go

Line 820 in 4f0bbba

func writeOCSPCacheFile() {

Hello, there's some inconsistent error messages in writeOCSPCacheFile():

	err := os.Mkdir(cacheLockFileName, 0600)
	switch {
	case os.IsExist(err):
		statinfo, err := os.Stat(cacheLockFileName)
		if err != nil {
			logger.Debugf("failed to write OCSP response cache file. file: %v, err: %v. ignored.\n", cacheFileName, err)
			return
		}
		if time.Since(statinfo.ModTime()) < 15*time.Minute {
			logger.Debugf("other process locks the cache file. %v. ignored.\n", cacheFileName)
			return
		}
		if err = os.Remove(cacheLockFileName); err != nil {
			logger.Debugf("failed to delete lock file. file: %v, err: %v. ignored.\n", cacheLockFileName, err)
			return
		}
		if err = os.Mkdir(cacheLockFileName, 0600); err != nil {
			logger.Debugf("failed to delete lock file. file: %v, err: %v. ignored.\n", cacheLockFileName, err)
			return
		}
	}

The first two might be more about showing the high-level intentions of the code rather than the lower-level indicator of what exactly failed, but in my experience, trying to give higher-level descriptions of failures is more confusing than helpful:

The first logger.Debugf() reports a failure to write the cache file, but what actually failed was a stat(2) on the lock file. The wrong filename is given in the error message.

The second logger.Debugf() reports the name of the cache file, not the lock file, despite the operation being on the lock file. The wrong filename is given in the error message.

The fourth logger.Debugf() reports the wrong operation.

I'm not accustomed to seeing lock directories used; especially since all the log messages here are discussing lock files. There's two very similar variables in play here, cacheFileName and cacheLockFileName and I'm worried that some of these operations may be using the wrong one.

Also left unhandled is if the original os.Mkdir(cacheLockFileName, 0600) fails for other reasons: permission denied, operation not permitted, I/O error, too many open files, etc.

Thanks

[setharnold]

Hmm, has the automation run amok here? I don't see a change in the most recent version, where the Mkdir on line 843 is called a delete operation in the log message:

gosnowflake/ocsp.go

Line 820 in 435e805

func writeOCSPCacheFile() {

Thanks

[sfc-gh-dszmolka]

hi, thank you for submitting this issue with us ! indeed the automation went a little bit amok last summer, but it has been rectified and this issue reopened.
which is still valid even today, so we'll take a look. appreciated the detailed description and thank you for bearing with us until this is taken care of.

[sfc-gh-dszmolka]

fix is merged and will be part of April's release, expected to come towards the end of the month

[sfc-gh-dszmolka]

fix released with 1.6.20