Azure / Azure/azure-libraries-for-java

Improving Exceptions bubbling

Aperta
#1,397 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Mgmt
Lingua principale
Java
Stelle
97
Fork
102
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

**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>> 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, Observable>>>() {
@Override
public Observable>> call(Response response) {
try {
ServiceResponse> result = listDelegate(response);
return Observable.just(new ServiceResponse>(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.blockForSingle(Observable) line: 463
BlockingObservable.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. https://github.com/ReactiveX/RxJava/issues/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 list() {
try {
ServiceResponse> response = listSinglePageAsync().toBlocking().single();
} catch (RuntimeException e) {
throw new RuntimeException("Ouch", e);
}
return new PagedList(response.body()) {
@Override
public Page nextPage(String nextPageLink) {
return listNextSinglePageAsync(nextPageLink).toBlocking().single().body();
}
};
}

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia con rx.exceptions.Exceptions.propagate(Throwable), BlockingObservable.blockForSingle/single() e ResourceGroupsInner.listSinglePageAsync()/list(), usando i percorsi di chiamata riportati come punto di confronto. Determina se il contesto dell’eccezione debba appartenere a RxJava o al client Azure; per completare il lavoro è necessario aver deciso l’ambito e avere una copertura di regressione mirata per il comportamento scelto.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java
Ambito
api, backend-api-design
Tipo di issue
Funzionalità
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Ferma
Chiarezza
Da chiarire
Idoneità per principianti
25/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.