mixed nil return with ErrNotFound

[woutslakhorst]

make up our minds on returning nil, nil or nil, ErrNotFound when nothing is found.

[stevenvegt]

With Update an ErrNotFound is very approprate. Then for consistancy I would like a (nil, ErrNotFound) for a Get as well.
It would be easier to write API middleware to catch the error and generate a 404 http error.
The check would also be more elegant:

doc, err := store.Get(id)
if err != nil {
  return doc, err
}

vs

doc, err := store.Get(id)
if err != nil || doc == nil {
  return doc, err
}

[reinkrul]

As a rule of thumb I just follow the intention of the function;

• "Get" implies the thing is expected to be there, so should lead to an error
• "Find" (single, or multiple results) implies it might not be there, so should not lead to an error

Problem for me (in Go) is that I often see code checking for the ErrNotFound error, then having different behaviour as part of normal application flow. For me errors are like Exceptions, thus should be an exceptional situation. So it shouldn't lead up to something like;

func createOrUpdate(doc Document) error {
     existing, err := store.Get(doc.ID)
     if err.is(ErrNotFound) {
        return store.Create(doc)
     } else if err != nil {
        return err
     }
    // Existing, so update
    return store.Update(doc)
}

Imo errors used for normal application flow leads to ugly code (err being checked twice). So in this case the store should've provided an exists func instead.

[reinkrul]

It would be easier to write API middleware to catch the error and generate a 404 http error.

There's always mapping code in the REST layer which can determine which error to return.

[woutslakhorst]

Final decision: error type checking remains in API functions. Moving it to middleware won't help and makes testing harder. It's also no longer possible to handle a 400 differently than a 404. So for now we'll keep checking for ErrNotFound in the API handler function.