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

configstore: no error when opening a config store that does not exist #128

Closed
awilliams-fastly opened this issue Jul 25, 2024 · 1 comment

Comments

@awilliams-fastly
Copy link

awilliams-fastly commented Jul 25, 2024

In the following sample code, no error is returned from configstore.Open("DOES_NOT_EXIST"), but a configstore.ErrStoreNotFound error is returned from cs.Get("cat").

cs, err := configstore.Open("DOES_NOT_EXIST") // err == nil
switch {
case errors.Is(err, configstore.ErrStoreNotFound):
        fsthttp.Error(w, "config store not found: configstore.Open", fsthttp.StatusNotFound)
        return
case err != nil:
        fsthttp.Error(w, fmt.Sprintf("error opening config store: %s", err.Error()), fsthttp.StatusInternalServerError)
        return
}

v, err := cs.Get("cat") // err == configstore.ErrStoreNotFound
switch {
case errors.Is(err, configstore.ErrStoreNotFound):
        fsthttp.Error(w, "config store not found: cs.Get", fsthttp.StatusNotFound)
        return
case errors.Is(err, configstore.ErrKeyNotFound):
        fsthttp.Error(w, "config store key not found", fsthttp.StatusNotFound)
        return
case err != nil:
        fsthttp.Error(w, fmt.Sprintf("error opening config store key: %s", err.Error()), fsthttp.StatusInternalServerError)
        return
}

To better match the Rust SDK, and to provide a better user experience, consider returning a configstore.ErrStoreNotFound from configstore.Open when the named config store does not exist.

Possibly related: #118

@dgryski
Copy link
Member

dgryski commented Aug 22, 2024

The current configstore package uses the old hostcalls, which do not return an error for missing stores.

https://github.com/fastly/compute-sdk-go/blob/main/internal/abi/fastly/dictionary_guest.go#L62

So indeed the correct path forward is to switch to the new hostcalls.

dgryski added a commit that referenced this issue Sep 3, 2024
@cee-dub cee-dub closed this as completed in 653c0f9 Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants