Fix errcheck issues in server/channels/app/import_utils_test.go

hanzei commented 2 hours ago

At Mattermost, we are committed to maintaining a high standard of code quality, reliability, and security. To further enhance this, we are launching a community campaign to integrate the errcheck linter across the Mattermost codebase.

This Help Wanted ticket is about fixing the linter issues reported by errcheck in channels/app/import_utils_test.go.

What is `errcheck`?

errcheck is a tool that automatically identifies places in Go code where errors are returned but not checked. In Go, error handling is a critical part of writing robust applications, and ignoring returned errors can lead to bugs, crashes, or unintended behaviour. By integrating errcheck into the codebase, we can ensure that error handling is consistently and properly managed across the project.

Tasks

Remove the exception for channels/app/import_utils_test.go from the exceptions list for errcheck in server/.golangci.yml.
Run make golangci-lint to get a list of all linter errors.
Fix the errors by properly handling them.
Run make golangci-lint again to ensure all errors are fixed.

Okay, but what does "properly handling" mean?

In general, errors should be returned to the caller. Non critical errors should be loggged, and function execution should continue. There are some exceptions to this rule, such as when the error is expected and should be ignored or when the error is handled in a different way. In such cases, a comment should be added to explain why the error is ignored.

In tests, errors should be checked using require.NoError(t, err) for errors of type error or require.Nil(t, err) for errors of type **model.AppError. If an error is expected, it should be checked using require.Error(t, err) for errors of type error or require.NotNil(t, err) for errors of type **model.AppError.

Use err as the variable name for errors and appErr for *model.AppError. Avoid using variable names like err1, err2, etc. See the Mattermost style guide for more information.

https://github.com/mattermost/mattermost/pull/28756 and https://github.com/mattermost/mattermost/pull/28767 can be used as examples of how to fix errcheck issues in the api4 package.

If you're interested, please comment here and come join our "Contributors" community channel on the Mattermost Community server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers Server" community channel. You can also reach out to @hanzei either on this ticket or on the Mattermost Community server.

For new contributors, please see our Developer's Guide.

If this ticket is already taken, please check out our other Help Wanted tickets of this campaign.

Thank you for your contributions to Mattermost!

JIRA: MM-61259

Gesare5 commented 2 hours ago

Can I give this a try? @hanzei

Bhabuk10 commented 1 hour ago

@hanzei could you assign me this issue ?

hanzei commented 1 hour ago

Sure thing @Gesare5 :+1: Please let me know if you have any questions.

mattermost / mattermost