Improving Exceptions bubbling #1397
Description
Is your feature request related to a problem? Please describe.
In many places, any sort of exceptions is bubbled without a context, leaving the stack-trace reader with a partial stack difficult to process
Describe the solution you'd like
I would like Exceptions to be properly chained, so that finally reported Exceptions holds a proper chain of Cause, enabling retracing the whole chain of calls.
Sorry for the quite long ticket while I suspect it may be rejected right-away, I tried to be quite exhaustive in my analysis.
Typically, I consider a piece of code like:
com.microsoft.azure.management.Azure azure = ...;
azure .resourceGroups().list()
I end with a failure in ResourceGroupsInner:
public Observable<ServiceResponse<Page<ResourceGroupInner>>> listSinglePageAsync() {
if (this.client.subscriptionId() == null) {
throw new IllegalArgumentException("Parameter this.client.subscriptionId() is required and cannot be null.");
}
if (this.client.apiVersion() == null) {
throw new IllegalArgumentException("Parameter this.client.apiVersion() is required and cannot be null.");
}
final String filter = null;
final Integer top = null;
return service.list(this.client.subscriptionId(), filter, top, this.client.apiVersion(), this.client.acceptLanguage(), this.client.userAgent())
.flatMap(new Func1<Response<ResponseBody>, Observable<ServiceResponse<Page<ResourceGroupInner>>>>() {
@Override
public Observable<ServiceResponse<Page<ResourceGroupInner>>> call(Response<ResponseBody> response) {
try {
ServiceResponse<PageImpl<ResourceGroupInner>> result = listDelegate(response);
return Observable.just(new ServiceResponse<Page<ResourceGroupInner>>(result.body(), result.response()));
} catch (Throwable t) {
return Observable.error(t);
}
}
});
}
This leads to stacks like:
2021-05-11 12:33:19.259 ERROR 2928 --- [nio-3000-exec-9] MyCustomExceptionHandler : Runtime error : Status code 403, {"error":{"code":"AuthorizationFailed","message":"The client 'XXX with object id 'XXX' does not have authorization to perform action 'Microsoft.Resources/subscriptions/resourcegroups/read' over scope '/subscriptions/YYY' or the scope is invalid. If access was recently granted, please refresh your credentials."}}
com.microsoft.azure.CloudException: Status code 403, {"error":{"code":"AuthorizationFailed","message":"The client 'XXX' with object id 'XXX' does not have authorization to perform action 'Microsoft.Resources/subscriptions/resourcegroups/read' over scope '/subscriptions/YYY' or the scope is invalid. If access was recently granted, please refresh your credentials."}}
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at com.microsoft.rest.ServiceResponseBuilder.build(ServiceResponseBuilder.java:122)
at com.microsoft.azure.AzureResponseBuilder.build(AzureResponseBuilder.java:56)
at com.microsoft.azure.management.resources.implementation.ResourceGroupsInner.listDelegate(ResourceGroupsInner.java:973)
at com.microsoft.azure.management.resources.implementation.ResourceGroupsInner.access$600(ResourceGroupsInner.java:49)
at com.microsoft.azure.management.resources.implementation.ResourceGroupsInner$31.call(ResourceGroupsInner.java:850)
at com.microsoft.azure.management.resources.implementation.ResourceGroupsInner$31.call(ResourceGroupsInner.java:846)
at rx.internal.operators.OnSubscribeMap$MapSubscriber.onNext(OnSubscribeMap.java:69)
at retrofit2.adapter.rxjava.CallArbiter.deliverResponse(CallArbiter.java:120)
at retrofit2.adapter.rxjava.CallArbiter.emitResponse(CallArbiter.java:102)
at retrofit2.adapter.rxjava.CallEnqueueOnSubscribe$1.onResponse(CallEnqueueOnSubscribe.java:41)
at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:129)
at okhttp3.RealCall$AsyncCall.execute(RealCall.java:174)
at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
My main issue here is that I capture an Exception is some sort of generic Exception handler (e.g. a Spring @ControllerAdvice). However, the stack does not even report my class having called azure .resourceGroups().list().
A richer stack would be available by wrapping the bubble-ed exception in ResourceGroupsInner.listSinglePageAsync :
return Observable.error(new RuntimeException(t));
However, this would not produce a full stack due to :
rx.exceptions.Exceptions.propagate(Throwable) used by BlockingObservable:
In my case, the stack looks like:
Daemon Thread [http-nio-3000-exec-1] (Suspended (exception CloudException))
owns: NioEndpoint$NioSocketWrapper (id=432)
Exceptions.propagate(Throwable) line: 53
BlockingObservable<T>.blockForSingle(Observable<? extends T>) line: 463
BlockingObservable<T>.single() line: 340
ResourceGroupsInner.list() line: 766
ResourceGroupsImpl.list() line: 41
.....
Hence, the calling thread own stack is not properly bubbled due to:
public static RuntimeException propagate(Throwable t) {
if (t instanceof RuntimeException) {
throw (RuntimeException) t;
} else if (t instanceof Error) {
throw (Error) t;
} else {
throw new RuntimeException(t); // NOPMD
}
}
The questions of exception bubbling in Rxjava is regularly considered (e.g. ReactiveX/RxJava#969).
However, blocking statements are considered inappropriate for production:
BlockingObservable:
It can be
* useful for testing and demo purposes, but is generally inappropriate for production applications (if you
* think you need to use a {@code BlockingObservable} this is usually a sign that you should rethink your
* design).
Should I consider this as an issue in Rxjava BlockingObservable ? (i.e. BlockingObservable does not propagate properly exceptions from different threads, hence losing the stack for calling thread?)
Or should I consider this should be managed by calling libraries (here Azure libraries), to catch systematically such exceptions, and re-wrap them? However, this would lead to the inner exception to be be available directly (still being available through the chain of causes).
In the second case, it would lead in any sync call to something like:
ResourceGroupsInner:
public PagedList<ResourceGroupInner> list() {
try {
ServiceResponse<Page<ResourceGroupInner>> response = listSinglePageAsync().toBlocking().single();
} catch (RuntimeException e) {
throw new RuntimeException("Ouch", e);
}
return new PagedList<ResourceGroupInner>(response.body()) {
@Override
public Page<ResourceGroupInner> nextPage(String nextPageLink) {
return listNextSinglePageAsync(nextPageLink).toBlocking().single().body();
}
};
}