Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise returned from AsyncHttpClient request doesn't handle exceptions thrown in then function #260

Closed
Qudratillo opened this issue Dec 13, 2022 · 4 comments

Comments

@Qudratillo
Copy link

Qudratillo commented Dec 13, 2022

fun get() {
  val httpClient = AsyncHttpClient.create(eventloop).withKeepAliveTimeout(Duration.ofHours(1))
  val promise = httpClient.request(HttpRequest.get(url)).then { response ->
   response.loadBody().map {
        try {
          throw RuntimeException("test exception")
        } catch( e: Exception) {
          throw e
        }
    }
  }
  return promise.map {response, ex -> if (ex != null) HttpResponse.ofCode(400) else response}
}

Above function (in Kotlin) expected to return HttpResponse.ofCode(400) but its promise is never completing since exception thrown in map block is not deligated to promise.
After putting try-catch block into then block I found that exception thrown in map is not thrown inside map block but it is thrown inside then block.

Stack trace


	at io.activej.promise.AbstractPromise$12.accept(AbstractPromise.java:604)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:103)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.SettablePromise.set(SettablePromise.java:68)
	at io.activej.http.HttpClientConnection.onHeadersReceived(HttpClientConnection.java:214)
	at io.activej.http.AbstractHttpConnection.readBody(AbstractHttpConnection.java:408)
	at io.activej.http.AbstractHttpConnection.readHeaders(AbstractHttpConnection.java:274)
	at io.activej.http.AbstractHttpConnection.readStartLine(AbstractHttpConnection.java:244)
	at io.activej.http.HttpClientConnection.readMessage(HttpClientConnection.java:126)
	at io.activej.http.AbstractHttpConnection$1.thenRun(AbstractHttpConnection.java:101)
	at io.activej.http.AbstractHttpConnection$ReadConsumer.accept(AbstractHttpConnection.java:595)
	at io.activej.http.AbstractHttpConnection$ReadConsumer.accept(AbstractHttpConnection.java:568)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.SettablePromise.set(SettablePromise.java:68)
	at io.activej.net.socket.tcp.AsyncTcpSocketNio.onReadReady(AsyncTcpSocketNio.java:374)
	at io.activej.eventloop.Eventloop.onRead(Eventloop.java:883)
	at io.activej.eventloop.Eventloop.processSelectedKeys(Eventloop.java:630)
	at io.activej.eventloop.Eventloop.run(Eventloop.java:543)
	at io.activej.service.adapter.ServiceAdapters$10.lambda$start$0(ServiceAdapters.java:282)

I expect any then and map functions of any Promise implementation must delegate exception thrown inside their map blocks to the promise itself. I am not sure if I can produce it in Java though.

@Qudratillo
Copy link
Author

I have made a minimal test

@Test
fun testPromiseRunTimeException() {
    val promise = SettablePromise<String>()
    promise.map { _ -> throw RuntimeException() }.map { _, _ -> "Good" }
    promise.set("test")
}

promise.set("test") throws RuntimeException

java.lang.RuntimeException
	at com.example.PromiseTest$testPromiseRunTimeException$1.invoke(PromiseTest.kt:10)
	at com.example.PromiseTest$testPromiseRunTimeException$1.invoke(PromiseTest.kt:10)
	at com.example.PromiseTest$sam$io_activej_common_function_FunctionEx$0.apply(PromiseTest.kt)
	at io.activej.promise.AbstractPromise$1.accept(AbstractPromise.java:196)
	at io.activej.promise.AbstractPromise.complete(AbstractPromise.java:114)
	at io.activej.promise.SettablePromise.set(SettablePromise.java:68)
	at com.example.PromiseTest.testPromiseRunTimeException(PromiseTest.kt:11)

@eduard-vasinskyi
Copy link
Contributor

Hi, @Qudratillo

ActiveJ has a fatal error handler facility called FatallErrorHandler. When a fatal error occurs inside a promise, the configured fatal error handler for a current thread handles it.
The default error handler is FatalErrorHandler.rethrow(), it rethrows any fatal error that occurs inside a promise. However, you can change the error handler for the current thread or eventloop.

In your last example, you use promises with no Eventloop. So, to change a fatal error handler you can add

FatalErrorHandlers.setThreadFatalErrorHandler(FatalErrorHandler.ignore());

at the start of a test. There are plenty of error handlers you can find in the FatalErrorHandler interface, or you can always write your own.

As for your initial issue, you use promises in the context of AsyncHttpClient which runs inside the Eventloop thread. To change the fatal error handler there you need to call eventloop#withThreadFatalErrorHandler(FatalErrorHandler) when instantiating an Eventloop. If you set the FatalErrorHandler.ignore() handler, for example, a promise would be completed with an exception.

@Qudratillo
Copy link
Author

Hi @eduard-vasinskyi,
I have tried eventloop#withThreadFatalErrorHandler(FatalErrorHandler). It worked sometimes, sometimes not.
I made some workaround

fun <T> Promise<T>.ingestNextExceptions(): Promise<T> {
    val promise = SettablePromise<T>()
    whenComplete { result, exception ->
        try {
            promise.accept(result, exception)
        } catch (e: Exception) {
            promise.exception = e
        }
    }
    return promise
}

Usage is

http.request(HttpRequest.post(url).withBody(requestBody))
    // need to call ingest since fatal errors sometimes cause trouble
  .ingestNextExceptions()
  .then { response -> 
      response.loadBody() }.map { body ->parse(body, typeReference) }
  }

@Qudratillo
Copy link
Author

It is solved using

FatalErrorHandlers.setGlobalFatalErrorHandler(FatalErrorHandler.ignore())
eventloop.withEventloopFatalErrorHandler(FatalErrorHandler.ignore()).withThreadFatalErrorHandler(FatalErrorHandler.ignore())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants