search

Icinga / icinga-notifications

Icinga Notifications: new and improved notifications and incident management for Icinga (work in progress, not ready for production yet)
GNU General Public License v2.0
10 stars 0 forks source link

Consistently wrap errors #115

Closed julianbrost closed 4 months ago

julianbrost commented 1 year ago

During the review of #99, I noticed there's already quite a bit of error handling in internal/incident/incident.go that returns entirely new errors instead of wrapping the errors that caused that error condition.

I don't see a reason why we wouldn't want to wrap errors there, but maybe @yhabteab does (pinging you as you wrote large parts of that code). If not, we should go over that code and adapt the error handling (I only noticed it in this file so far, but scrolling over all uses of errors.New() and fmt.Errorf() without %w probably doesn't hurt).

yhabteab commented 1 year ago

I guess I did this purely as we already log the actual error, it just makes it uglier to reveal any internal database query errors further out to the clients. They just need to be aware that something went wrong. Icinga 2 also does not disclose all API request failures, what the actual error was.

julianbrost commented 1 year ago

That's something that should probably be changed as well anyways: HTTP responses should only contain something like "error while processing request, check server logs for details".