Mattermost is migrating all usages of the AppError type to the idiomatic error type. AppError is a loaded error type containing fields like HTTP Status code which is not really necessary for errors coming from the DB layer. We should migrate to using normal types satisfying the error interface.
This ticket is about migrating all usages of AppError from store/sqlstore/cluster_discovery_store.go to return a normal error. The migration should use the error types defined in store/errors.go. If the need should arise to add another specific error type, feel free to add it. And then in the app layer, we would need to convert the error type to the usual AppError. This means we need a switch case to check for the type of error and appropriately convert to the correct AppError. It can be something like:
if _, err := a.Srv().Store.Bot().Update(bot); err != nil {
var nfErr *store.ErrNotFound
var appErr *model.AppError
switch {
case errors.As(err, &nfErr):
return model.MakeBotNotFoundError(nfErr.Id)
case errors.As(err, &appErr): // in case we haven't converted to plain error.
return appErr
default: // last fallback in case it doesn't map to an existing app error.
return model.NewAppError("DeleteBotIconImage", "app.bot.patchbot.internal_error", nil, err.Error(), http.StatusInternalServerError)
}
}
Example Pull Request: #14339
If you have questions about the ticket or you need any help, feel free to contact agniva.de.sarker in https://community.mattermost.com/
If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.
New contributors please see our Developer's Guide.
JIRA ticket: https://mattermost.atlassian.net/browse/MM-25312
@agnivade I would like to work on this
All yours !
I did not have time yet to work on this issue due to my university related homeworks but hope to have pr ready this weekend.
Thanks for the update @kadir96 !
Hey @kadir96 - how is it going with this one? :) Please let us know if there's anything we can do to help out. Thanks !
@kadir96 - Just checking in on this one. Is this still on your radar?
@kadir96 - I see that @rvillablanca has submitted a PR for this. I am going to assign it to him.