Refining exception throws

[junghoon-vans]

Description

Currently, the exceptions thrown are all MeilisearchException. This makes it difficult for the user to specify which exception was thrown.

Basic example

The way to fix this is to throw all exceptions as-is. Instead, the practice of creating and throwing MeilsearchException directly should be eliminated.

AS-IS

public <T> HttpResponse<T> execute(HttpRequest request) throws MeilisearchException {
    try {
        Request okRequest = buildRequest(request);
        Response response = client.newCall(okRequest).execute();

        return buildResponse(response);
    } catch (MalformedURLException e) {
        throw new MeilisearchException(e); // do not throw MeilisearchException directly!
    } catch (SocketTimeoutException e) {
        throw new MeilisearchTimeoutException(e);
    } catch (IOException e) {
        throw new MeilisearchCommunicationException(e);
    }
}

If the user wants to get a specific exception, they have to compare if it is castable and perform the cast.

TO-BE

public <T> HttpResponse<T> execute(HttpRequest request)
    throws MeilisearchURLException, MeilisearchTimeoutException, MeilisearchCommunicationException {
    try {
        Request okRequest = buildRequest(request);
        Response response = client.newCall(okRequest).execute();

        return buildResponse(response);
    } catch (MalformedURLException e) {
        throw new MeilisearchURLException(e); // for example
    } catch (SocketTimeoutException e) {
        throw new MeilisearchTimeoutException(e);
    } catch (IOException e) {
        throw new MeilisearchCommunicationException(e);
    }
}

With this change, users will be able to receive verbose exceptions(e.g. MeilisearchURLException..) as they are, or as a MeilisearchException as they do now.

Other This issue has great synergy when applied with #677. If all exceptions were RuntimeException, we wouldn't need any code to re-throw.

public <T> HttpResponse<T> execute(HttpRequest request) { // we don't have to re-throw!
    try {
        Request okRequest = buildRequest(request);
        Response response = client.newCall(okRequest).execute();

        return buildResponse(response);
    } catch (MalformedURLException e) {
        throw new MeilisearchURLException(e); // for example
    } catch (SocketTimeoutException e) {
        throw new MeilisearchTimeoutException(e);
    } catch (IOException e) {
        throw new MeilisearchCommunicationException(e);
    }
}

[curquiza]

Hello @junghoon-vans

thanks for the suggestion

Feel free to open PRs so that we better see what you mean. I'm not sure I like the MeilisearchURLException because the meaning is not explicit 😅 but I trust you for the global change!

Feel free to tackle https://github.com/meilisearch/meilisearch-java/issues/677 too

[junghoon-vans]

@curquiza Thank you for believing in me. I've been trying to figure out how to make this change, and it's been a while. For now, I've only raised a PR for issue #677.

[239yash]

@curquiza Hi, Can I take a hit on this. I was thinking to revamp all the exception throws in the code.

[curquiza]

Hello @239yash

Feel free to take this 😊 thank you very much!

For your information, we prefer not assigning people to our issues because sometimes people ask to be assigned and never come back, which discourages the volunteer contributors from opening a PR to fix this issue. We will accept and merge the first PR that fixes correctly and well implements the issue following our contributing guidelines.

[239yash]

@curquiza I am implementing this. Thanks!

[239yash]

@curquiza Please review the PR & do let me know of any changes to be made. Thanks