TimeleapLabs/unchained

Fix: Proper error handling

Closed this issue · 6 comments

There are a lot of places in the code where we just panc(err). That shouldn't be the case; we need to handle these errors properly.

We know that all errors should not close our app with os error. There are two approaches to dealing with errors:

first, top-down:

  • log errors when they happen, it gives the ability to have the source of them, and after that return a customized internal error (it is better to aggregate all custom errors in one place)
if err != nil {
   log.Error(err)
   return errors.New("something is wrong")
}

second way, down-top:
return all errors by wrapping them with some extra information with errors.wrap(err, "some extra data"). and when they come to the nearest layer to the endpoint, convert them to customized errors or show them when we are in development mode.

I suggest the first one.

Let's go with the top-down approach.

@harpy-wings this should be done after merging #84, you should branch out from #84. You'll have to rebase after the merge if you branch out from develop.

we should use Error Wrapping,

err := doSomething()
if err != nil {
  return errors.Join(err,errors.New("unexpected failure))
}

note that it may not be required to use the same thing everywhere. sometimes a simple return is better.
we should wrap errors while we are changing the layers, for example from the service layer to application gateways it's better to be wrapped.

also, there is a dependency on refactoring since an error should be returned instead of panic or log. and we need to change the function's interface for all of the packages.
@logicalangel let me know what you think and how we should approach this.

duo to #107 all panic are replaced, but we should consider error handling in the next PRs.