Skip to content

Conversation

@dvaldivia
Copy link
Collaborator

This PR renames the error type we used called Error to ApiError to avoid ambiguity. It also removes the redundant Code field as the http response always has a statusCode.

This results in much simpler code written both on Go and Typescript.

After removing the ambiguity, code in Typescript can actually rely on type inference from the generated swagger API since we no longer have ambiguity with Error, this also means we have at least 3 less imports per component file at the top.

Before

api.policies
      .listPolicies()
      .then((res: HttpResponse<ListPoliciesResponse, ApiError>) => {
        const policies = res.data.policies ?? [];
        isLoading(false);
        setRecords(policies.sort(policySort));
      })
      .catch((err: ErrorResponseHandler) => {
        isLoading(false);
        dispatch(setModalErrorSnackMessage(err));
      });

After

api.policies
      .listPolicies()
      .then((res) => {
        const policies = res.data.policies ?? [];
        isLoading(false);
        setRecords(policies.sort(policySort));
      })
      .catch((err) => {
        isLoading(false);
        dispatch(setModalErrorSnackMessage(err));
      });

@dvaldivia dvaldivia self-assigned this Jul 19, 2023
@harshavardhana harshavardhana changed the title Rename Error to ApiError to avoide ambiguity Rename Error to ApiError to avoid ambiguity Aug 1, 2023
@kaankabalak
Copy link
Contributor

kaankabalak commented Aug 14, 2023

Hi @dvaldivia, the lint check seems to be failing with the following messages:

Error: restapi/user_objects.go:43:12: could not import github.com/minio/console/restapi/operations/object (-: # github.com/minio/console/restapi/operations/object
Error: restapi/operations/object/download_multiple_objects_responses.go:88:18: undefined: models.Error
Error: restapi/operations/object/download_multiple_objects_responses.go:1[14](https://github.com/minio/console/actions/runs/5852265254/job/15864218789?pr=2954#step:4:15):70: undefined: models.Error
Error: restapi/operations/object/download_multiple_objects_responses.go:120:69: undefined: models.Error) (typecheck)
	objectApi "github.com/minio/console/restapi/operations/object"
	          ^
Error: restapi/user_objects.go:640:145: undefined: models.Error (typecheck)
func getMultipleFilesDownloadResponse(session *models.Principal, params objectApi.DownloadMultipleObjectsParams) (middleware.Responder, *models.Error) {
                                                                                                                                                ^
Error: restapi/user_objects.go:119:19: undefined: models.Error (typecheck)
		var err *models.Error
		                ^
Error: restapi/user_objects_test.go:1458:[17](https://github.com/minio/console/actions/runs/5852265254/job/15864218789?pr=2954#step:4:18): undefined: models.Error (typecheck)
		want1 *models.Error
		              ^

…tus code

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
@dvaldivia
Copy link
Collaborator Author

fixed @kaankabalak

Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dvaldivia dvaldivia merged commit 912a4b2 into minio:master Aug 16, 2023
@dvaldivia dvaldivia deleted the api-error-rename branch August 16, 2023 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants