Remove ModelWithScope and add handleError

[DaddyWarbucks]

This PR extends the class to have a handleError function that will make it easier for the user to handle errors from the service. It also removes ModelWithScope in favor of getModel. The params.sequelize.scope feature was never really documented. I have updated the README to reflect how to regain this functionality and more.

Closes
#433
#449

[fratzinger]

Thanks for the PR. Thanks for the review invite. Direction is good. I left my two cents for handleError. Curious what you think about it.

[fratzinger]

You could remove the optional chaining (?.). I think, if you use the chains, ts complains that you can't pass undefined to Model.scope() even though it should know that the optional chaining is useless.

It's just docs and everyone should be capable to fix this for themselves, but yeah.

[fratzinger]

same

[fratzinger]

It's really really important for handleError or errorHandler to throw an error. If you overwrite handleError with a function that does not throw anything, you swallow the error easily.

We call that function in every .catch() clause. We just should be double sure, that the error cannot be swallowed. So at least we should add a private _handleError function or something that calls the overwritable handleError function and throws the error at the end. If handleError throws an error, our fallback error won't be reached.

In addition the default errorHandler is pretty convenient. With a custom handler it's easy to mess things up. What if errorHandler already receives the wrapped error from errorHandler? Does this fit your use case? Or is a signature handleError(feathersError: any, sequelizeError: any) conceivable?

[DaddyWarbucks]

This is good feedback. I intended to use it like this

handleError(error) {
  if (error.message regex something) {
    error.message = "Safe Error message"
  }
  return super.handleError(error);
}

But you are right. Other users may not handle and re-throw the error and it would be swallowed. I like the idea of wrapping our use in another handler. No need to attach it to the class as _handleError. We will just keep a private function and wrap our own usage of handleError.

[DaddyWarbucks]

@fratzinger This has been updated with your suggestions. The double try/catch thing in catchHandler is a little goofy. So I stared to update errorHandler to be convertError and return an error instead of throw one. But then I realized errorHandler is exported from the package and is probably more useful as is than as a convertError. So even though the try/catch thing is a little goofy, I think its fine.

[DaddyWarbucks]

I also used GeneralError instead of Error in the few other places.

[DaddyWarbucks]

@fratzinger Any other thoughts here?

[fratzinger]

Released as v8.0.0-pre.2 🎉