Azure / Azure/azure-libraries-for-java

Improving Exceptions bubbling

Offen
#1,397 3 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Mgmt
Vorherrschende Sprache
Java
Sterne
97
Forks
102
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

**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();
}
};
}

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginnen Sie mit rx.exceptions.Exceptions.propagate(Throwable), BlockingObservable.blockForSingle/single() und ResourceGroupsInner.listSinglePageAsync()/list(), wobei Sie die gemeldeten Aufrufpfade als Vergleichspunkt verwenden. Bestimmen Sie, ob der Ausnahme-Kontext in RxJava oder dem Azure-Client liegen sollte; für den Abschluss müssen der Umfang festgelegt und eine fokussierte Regressionstestabdeckung für das gewählte Verhalten vorhanden sein.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
java
Bereich
api, backend-api-design
Issue-Typ
Feature
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Veraltet
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
25/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.