adelowo/onecache

Generates a open file error instead of a onecache.ErrCacheMiss

mikeschinkel opened this issue · 4 comments

return nil, err

I was going to submit a pull request but could not figure out exactly how to fix it given some issues I was running into getting it set up on GoLand so I could both edit and debug it (hard to edit and debug an external library in my IDE w/o renaming all the import paths.)

Anyway, without this is makes error handling harder than it needs to be.

Hi, Thanks... That error on the line is going to be an open file one. Kindly take a look at the Godoc of os#Open It is going to be an error of type *PathError.

Are you sure the item isn't expired ?

https://godoc.org/os#Open

What I was saying is that when I call store.Get("profile") from your README example that I do not expect to get a file open error after the cache has timed out.

Actually, the first time I call it I get an ErrCacheMiss error, but the second time I call store.Get() I get a file open error. That is inconsistent and requires error handled to take into consideration both ErrCacheMiss and file open error when in reality what is really happening from the caller's perspective is a cache miss.

The caller does not care that the cache file is no longer there, it is an implementation detail of your library.

Since I first posted this issue I decided to write my own very simple cache library because it was taking me more time to get what I needed trying to figure out how to patch yours (although this article you linked me will help me in the future — thanks for that) so I was able to implement the error handling code I am suggesting you consider implementing for your lines 116-118:

if err != nil {
	pe, ok := err.(*os.PathError)
	if !ok {
		return nil,err
	}
	if pe.Err == syscall.ENOENT && pe.Op == "open" {
		return nil,ErrCacheMiss
	}
	return nil,err
}

Hmm, when a cache miss occurs ( due to expiration ), the file is deleted. So calling it the second time will be an "open file" error.

Your fix is fine and acceptable behaviour. Do you want to send a PR for that?

Thanks for everything.

@adelowo Thanks. Sorry I did not get a chance to submit a PR. I had to put out some client fires.