embulk / embulk/embulk-base-restclient
OAuth 401 is throwing wrong exception due to bug of jetty-client 9.2.x
- Dominant language
- Java
- Stars
- 6
- Forks
- 7
- PR merge metrics
- No merged PRs in 30d
Description
### Problem description

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.