Setting @ErrorHandler doesn't catch SocketTimeoutException

[kenkyee]

Tested by adding the @ErrorHandler annotation before my class:

@Retrofit.ErrorHandler(MYErrorHandler.class)
public abstract class TestCreationApi {

My handler looks like the example but it never gets called:

public class MyErrorHandler implements ErrorHandler {
    @Override
    public Throwable handleError(RetrofitError cause) {
        Response r = cause.getResponse();
        if (r != null && r.getStatus() == 401) {
            // clear authToken because we've been logged out by the server
            AuthApiManager.setAuthToken(null);

            Log.d("MyErrorHandler", "Logged out because of bad token");

            return new RuntimeException("401", cause);
        }
        return cause;
    }
}

Breakpoints don't get hit in the code and the print statement doesn't seem to work. Generated code seem to set it though.

        if (this.errorHandler == null) {
            this.errorHandler = new com.serverapi.MYErrorHandler();
        }

It doesn't catch the 401 error or a sockettimeout. You can reproduce the sockettimeout by disconnecting your internet network temporarily on the emulator (genymotion) so that's easier to test than the 401.

[yongjhih]

It should be fixed: b2b66075fd

[kenkyee]

Doesn’t seem to work still…when I shut off wifi, the error handler’s handleError() never gets called…an exception happens here instead...

On Nov 12, 2015, at 4:46 PM, Andrew Chen notifications@github.com wrote:

It should be fixed: b2b6607 https://github.com/yongjhih/NotRetrofit/commit/b2b66075fdc455c66d37c1191d39ee12e83f3b23 — Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-156246092.

[kenkyee]

Forgot to mention I did add this to my gradle file:

compile 'com.github.yongjhih.NotRetrofit:retrofit:b2b6607' apt 'com.github.yongjhih.NotRetrofit:retrofit-processor:b2b6607'

On Nov 12, 2015, at 4:46 PM, Andrew Chen notifications@github.com wrote:

It should be fixed: b2b6607 https://github.com/yongjhih/NotRetrofit/commit/b2b66075fdc455c66d37c1191d39ee12e83f3b23 — Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-156246092.

[kenkyee]

[yongjhih]

Hi Ken,

Sorry for replying late, I was on holiday.

It should be fixed by 06d6be7.

[kenkyee]

Thanks. Hope you had a good holiday :-)

FWIW, I tried pinning to the latest: compile 'com.github.yongjhih.NotRetrofit:retrofit:06d6be7' apt 'com.github.yongjhih.NotRetrofit:retrofit-processor:06d6be7'

but my code in my error handler never runs. I think it’s because it gets no exception when the server sends a 401…it sends JSON back w/ an error msgs so it doesn’t cause any exceptions in your code so the error handler never gets calls. The error handler does get called if it can’t reach the server when I shut off wifi though, so I know the error handler is hooked in right.

ken

On Nov 17, 2015, at 4:10 AM, Andrew Chen notifications@github.com wrote:

Hi Ken,

Sorry for replying late, I was on holiday. I think you were using blocking method. I couldn't reproduce with non-blocking method such as Observable getXXX().

The blocking method must use try-catch avoid throw exception after ErrorHandler handled.

try { service.getXXX(); } catch (Throwable e) {} However, after 9928af7 https://github.com/yongjhih/NotRetrofit/commit/9928af72559ec5374156dc3bf4a659f6726b3780, You can use ErrorHanlder to ignore error with blocking method by the following:

public class GitHubErrorHandler implements ErrorHandler { @Override public Throwable handleError(RetrofitError cause) { Response r = cause.getResponse(); if (r != null && r.getStatus() == 401) { return new RuntimeException("401", cause); } return null; // ignore error } } — Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-157314557.

[yongjhih]

You are right, I also found it. So i committed 6de8721: Support httpError exception throwing.

[kenkyee]

I’m not getting an error code. I think I see a problem in your MainTest unit test. You’re checking that the error handler gets called, in your mock service, but you’re not checking that it’s passing through the response code. I see that when the error handler gets hit too…the response field is null.

With that checkin, I’m also finding that when I get an error response code, the JSON in the body of the response is no longer being parsed like it used to be. I can work around this, but it’s a different behavior than before. Unit tests are definitely a good idea :-)

Hope that helps.

ken

On Nov 17, 2015, at 2:01 PM, Andrew Chen notifications@github.com wrote:

You are right, I also found it. So i committed 6de8721 https://github.com/yongjhih/NotRetrofit/commit/6de8721057e1c3b1dfd2fc995e083047ff570593: Support httpError exception throwing.

— Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-157471415.

[yongjhih]

It's mistake that did not check response code in testHttpErrorHandler(). But I had checked the testing log printed that represented the right response code of 404 before, not null. I think you need to clean build after updated version.

And welcome any patch for unit tests to show the problem up.

[kenkyee]

Cleaning build didn’t help. I’ll try to write up a unit test for it and submit a PR :-)

thanks,

ken

On Nov 17, 2015, at 2:37 PM, Andrew Chen notifications@github.com wrote:

It's mistake that did not check response code in testHttpErrorHandler(). But I was checked the testing log that represented the right response code of 404 before, not null. I think you must clean build after updated version.

And welcome any patch for unit tests to show the problem up.

— Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-157482477.

[kenkyee]

Added unit tests and confirmed error handler gets called when using the MockWebServer. The unit tests show the issue of not getting the response body if the error handler is called w/ an error response code.

Not sure why it’s not acting the same way on my system w/ a real web server but will investigate more.

On Nov 17, 2015, at 2:37 PM, Andrew Chen notifications@github.com wrote:

It's mistake that did not check response code in testHttpErrorHandler(). But I was checked the testing log that represented the right response code of 404 before, not null. I think you must clean build after updated version.

And welcome any patch for unit tests to show the problem up.

— Reply to this email directly or view it on GitHub https://github.com/yongjhih/NotRetrofit/issues/6#issuecomment-157482477.

[yongjhih]

Thanks for PR, it has been merged #8 and fixed null responseBody by f0f93c42