ObsoleteUrlFactory overrides OkHttpClient’s interceptors

[Typraeurion]

Describe the bug
I’ve been troubleshooting why my custom network interceptor that I added to the OkHttpClient was never called. After tracing through the code, I found that ObsoleteUrlFactory.buildCall is explicitly removing all interceptors set on the OkHttpClient and adding its own:

            OkHttpClient.Builder clientBuilder = client.newBuilder();
            clientBuilder.interceptors().clear();
            clientBuilder.interceptors().add(UnexpectedException.INTERCEPTOR);
            clientBuilder.networkInterceptors().clear();
            clientBuilder.networkInterceptors().add(networkInterceptor);

To Reproduce
Steps to reproduce the behavior:

    public class TimingInterceptor implements Interceptor {

        @Override
        public Response intercept(Chain chain) throws IOException {
            System.out.println("Intercepting network call from OkHttp");
        }

    }

    public GHUser getCurrentUser(String authToken)
            throws ForbiddenException, IOException {

        OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder();
        clientBuilder.addNetworkInterceptor(new TimingInterceptor());
        OkHttpConnector httpConnector = new OkHttpConnector(clientBuilder.build());
        GitHub gitHub = new GitHubBuilder()
                .withConnector(httpConnector)
                .withOAuthToken(authToken)
                .build();
        GHMyself me = gitHub.getMyself();

        return me;

    }

Expected behavior
Application displays “Intercepting network call from OkHttp” when getCurrentUser is called.

Desktop (please complete the following information):

• Java: 1.8.0
• github-api: 1.333
• okhttp: 4.9.2 (previously tried 3.14.9, with the same result)

[bitwiseman]

Generally speaking, this is a known limitation of ObsoleteUrlFactory. This is another reason we need to implement #1085 so we can stop using the deprecated/obsolete code path.

You are welcome to take a swing at implementing a fix in ObsoleteUrlFactory, but I suspect there was a good reason why the original implementation does not allow that. If you choose to try fixing you will need to implement extensive testing in that area.

If you are interested in working on #1085, let's talk over there.

[bitwiseman]

@Typraeurion
We won't be fixing this in ObsoleteUrlFactory. In the next release (code already merged to main) we've added OkHttpGitHubConnector which does not depend on ObsoleteUrlFactory allowing for use of interceptors.

If you are using OkHttpConnector switch to OkHttpGitHubConnector.