search

timols / java-gitlab-api

A wrapper for the Gitlab API written in Java
Apache License 2.0
387 stars 317 forks source link

Exceptions should never be propagated as a generic Java Error #275

Closed mjpitz closed 6 years ago

mjpitz commented 6 years ago

This tends to cause processes to crash as no-one catches this type of error. From what I can tell, it looks like we are re-boxing the GitlabAPIexception with a java.lang.Error.

java.lang.Error: org.gitlab.api.GitlabAPIException: {"message":"403 Forbidden"}
    at org.gitlab.api.http.GitlabHTTPRequestor$1.fetch(GitlabHTTPRequestor.java:263)
    at org.gitlab.api.http.GitlabHTTPRequestor$1.hasNext(GitlabHTTPRequestor.java:217)
    at org.gitlab.api.http.GitlabHTTPRequestor.getAll(GitlabHTTPRequestor.java:185)
    at org.gitlab.api.GitlabAPI.getRepositoryTree(GitlabAPI.java:1563)
mjpitz commented 6 years ago

Looking at the code really quick, I see this double try catch causing the issue:

                try {
                    HttpURLConnection connection = setupConnection(url);
                    try {
                        next = parse(connection, type, null);
                        assert next != null;
                        findNextUrl();
                    } catch (IOException e) {
                        handleAPIError(e, connection);
                    }
                } catch (IOException e) {
                    throw new Error(e);
                }
timols commented 6 years ago

@jpitz you make a very good point. Would you be ok to submit a PR for this change?

mjpitz commented 6 years ago

I can look at putting one together. Will comment back once I have one.

On Dec 22, 2017, at 1:20 PM, Tim Olshansky notifications@github.com wrote:

@jpitz you make a very good point. Would you be ok to submit a PR for this change?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mjpitz commented 6 years ago

@timols : feel free to assign to me if possible. I should have something up today.

mjpitz commented 6 years ago

Pull request here: https://github.com/timols/java-gitlab-api/pull/281

jaredhodge commented 6 years ago

@timols ; how about spinning a new version of the library and propagating it out maven central so people can take advantage of this fix?

jaredhodge commented 6 years ago

I see 4.1.0 out there now. Thanks @timols!