embulk / embulk/embulk-base-restclient

OAuth 401 is throwing wrong exception due to bug of jetty-client 9.2.x

Open
#99 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
6
Forks
7
PR merge metrics
No merged PRs in 30d

Description

### Problem description
jetty_client_bug

Due to this bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=439448, `jetty-client` will throw a wrong `HttpResponseException` at: https://github.com/eclipse/jetty.project/blob/jetty-9.2.14.v20151106/jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java#L111

This causes several issues:
1. Exception passed to Embulk plugin [is wrapped](https://github.com/eclipse/jetty.project/blob/jetty-9.2.14.v20151106/jetty-client/src/main/java/org/eclipse/jetty/client/util/InputStreamResponseListener.java#L228
) as `ExecutionException`
2. Hence, it could cause unexpected retries, even with below (no retry for 401):
```java
@Override
protected boolean isResponseStatusToRetry(Response response)
{
int statusCode = response.getStatus();
return (statusCode == 429) || (statusCode / 100 != 4);
}
```
3. Thrown error message may contain stacktrace, which looks ugly on UI.

### Workaround
```java
@Override
protected boolean isExceptionToRetry(Exception exception)
{
// workaround: exception is wrapped in java.util.concurrent.ExecutionException
// not retry in this case
return !(exception instanceof ExecutionException && exception.getCause() instanceof HttpResponseException);
}
```
Not retry when exception is `ExecutionException` and its cause is `HttpResponseException`. Now it's a bit incorrect, but that's what I can come up for now, to avoid wrong retries.

### Expected result
`StringJetty92ResponseEntityReader` can read 401 error when response headers don't contain realm name.

According to bug tracking, it's easy to implement and override `ProtocolHandler` but not easy to replace default `AuthenticationProtocolHandler`.

The bug is fixed in `v9.3.x` though (requires Java 8).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.