bunq / sdk_java

Java SDK for bunq API
MIT License
47 stars 23 forks source link

How do I close Bunq API connection? #112

Open ChappIO opened 5 years ago

ChappIO commented 5 years ago

Steps to reproduce:

  1. Load an ApiContext
  2. Call BunqContext.loadApiContext(context)

What should happen:

  1. The application should close

What happens:

  1. A (non daemon) thread remains open

Traceback

SDK version and environment

public class Application {
    public static void main(String[] args) {
        // Load bunq context
        var context = loadBunq();
        BunqContext.loadApiContext(context);
    }

    private static ApiContext loadBunq() {
        var contextFile = new File(env("BUNQ_API_CONTEXT_FILE_PATH", "bunq.conf"));

        ApiContext context = contextFile.exists() ? ApiContext.restore(contextFile.getAbsolutePath()) : ApiContext.create(
                ApiEnvironmentType.valueOf(env("BUNQ_ENVIRONMENT_TYPE", "PRODUCTION")),
                env("BUNQ_API_KEY", ""),
                "Bunq to YNAB"
        );

        context.save(contextFile.getAbsolutePath());
        return context;
    }

    private static String env(String name, String def) {
        var result = System.getenv(name);
        if (result == null || result.isBlank()) {
            return def;
        }
        return result;
    }
}

I would expect this application to close after a bit. However, when I run it there are still a few threads there. I see no close methods on the context so how do I close the context?

ChappIO commented 5 years ago

If you can just give me a pointer, then I can create a pull request as well. No problem.

mhogerheijde commented 5 years ago

I have this problem as well. It is highly annoying. From my point of view a client should never keep a thread running unless explicitly asked for by the developer. I need to have control over my threads.

Also: this doesn't sparkle with the principle of least surprise.

mhogerheijde commented 5 years ago

Might this have to do with the HTTP/2 nature of the used http client? Since HTTP/2 allowes multiplexing connections, the client needs to keep the connection open to receive new responses. It is, however, annoying that there is no explicit close method on this.

Timeout is set on 30 seconds, the client, however does not error out after 30 seconds, so I'm not entirely sure here.

mhogerheijde commented 5 years ago

Update, client exits after 1 minute.

mhogerheijde commented 5 years ago

To force the application to exit, you can end your main method with System.exit(0).

mhogerheijde commented 5 years ago

@ChappIO The OkHttp client being used in the core of the client seems to be closeable. See also https://stackoverflow.com/questions/29055724/okhttpclient-close-connection

In effect the Bunq API client needs to call close on (all) the response bodie(s) it has a connection for. (response.body().close())