snowplow / snowplow-java-tracker

Snowplow event tracker for Java. Add analytics to your Java desktop and server apps, servlets and games. (See also: snowplow-android-tracker)
http://snowplowanalytics.com
Apache License 2.0
23 stars 36 forks source link

OkHttpClient return code is always = -1 when the response is code 404 or any other not successful code #366

Closed eusorov closed 10 months ago

eusorov commented 10 months ago

Describe the bug When OkHttpClient sends snowplow payload and it fails with any code like 404, then the return code is always = -1.
It only sets to actual value, when the response is successful.

To Reproduce OkHttpClientAdapter:

public int doPost(String url, String payload) {
        int returnValue = -1;

        RequestBody body = RequestBody.create(payload, JSON);
        Request request = new Request.Builder()
                .url(url)
                .addHeader("Content-Type", Constants.POST_CONTENT_TYPE)
                .post(body)
                .build();
        try (Response response = httpClient.newCall(request).execute()) {
            if (!response.isSuccessful()) {
                LOGGER.error("OkHttpClient POST Request failed: {}", response);
            } else {
                returnValue = response.code();
            }
        } catch (IOException e) {
            LOGGER.error("OkHttpClient POST Request failed: {}", e.getMessage());
        }

        return returnValue;
    }

Expected behavior in the block

if (!response.isSuccessful()) { } there should be also returnValue = response.code(); So for example retry etc can pick up the code and evaluate the value. better remove the else block like this:

                try (Response response = httpClient.newCall(request).execute()) {
            if (!response.isSuccessful()) {
                LOGGER.error("OkHttpClient POST Request failed: {}", response);
            }
            returnValue = response.code();
        } catch (IOException e) {
            LOGGER.error("OkHttpClient POST Request failed: {}", e.getMessage());
        }

Same applies for doGet() method Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

Additional context Add any other context about the problem here.