From 1bb1fcfd0e6af3083c6a9751d9795c8139147f51 Mon Sep 17 00:00:00 2001 From: Bart De Smet Date: Tue, 29 Sep 2020 20:59:44 -0700 Subject: [PATCH] Non-empty stack traces for operator exceptions. --- .../Linq/Observable/Aggregate.cs | 9 ++- .../Linq/Observable/Average.cs | 45 +++++++++++++-- .../Linq/Observable/ElementAt.cs | 9 ++- .../Linq/Observable/FirstAsync.cs | 18 +++++- .../Linq/Observable/LastAsync.cs | 18 +++++- .../System.Reactive/Linq/Observable/Max.cs | 54 ++++++++++++++++-- .../System.Reactive/Linq/Observable/Min.cs | 54 ++++++++++++++++-- .../Linq/Observable/SingleAsync.cs | 36 ++++++++++-- .../Linq/Observable/SingleOrDefaultAsync.cs | 18 +++++- .../Linq/QueryLanguage.Blocking.cs | 8 ++- .../Tasks/TaskObservableExtensions.cs | 9 ++- .../Tests/Linq/Observable/SingleAsyncTest.cs | 57 +++++++++++++++++++ 12 files changed, 304 insertions(+), 31 deletions(-) diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Aggregate.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Aggregate.cs index f560e0d752..58a116b97b 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Aggregate.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Aggregate.cs @@ -62,7 +62,14 @@ public override void OnCompleted() { if (!_hasAccumulation) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Average.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Average.cs index fd21022a16..76f63782ca 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Average.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Average.cs @@ -54,7 +54,14 @@ public override void OnCompleted() } else { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } @@ -110,7 +117,14 @@ public override void OnCompleted() } else { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } @@ -166,7 +180,14 @@ public override void OnCompleted() } else { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } @@ -222,7 +243,14 @@ public override void OnCompleted() } else { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } @@ -278,7 +306,14 @@ public override void OnCompleted() } else { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/ElementAt.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/ElementAt.cs index b2511add34..badf13ec92 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/ElementAt.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/ElementAt.cs @@ -44,7 +44,14 @@ public override void OnCompleted() { if (_i >= 0) { - ForwardOnError(new ArgumentOutOfRangeException("index")); + try + { + throw new ArgumentOutOfRangeException("index"); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/FirstAsync.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/FirstAsync.cs index c9eb700cc1..cafb8f82ed 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/FirstAsync.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/FirstAsync.cs @@ -39,7 +39,14 @@ public override void OnCompleted() { if (!_found) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } @@ -97,7 +104,14 @@ public override void OnCompleted() { if (!_found) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } } } diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/LastAsync.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/LastAsync.cs index 8530b26394..3bad6da8ce 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/LastAsync.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/LastAsync.cs @@ -46,7 +46,14 @@ public override void OnCompleted() { if (!_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -118,7 +125,14 @@ public override void OnCompleted() { if (!_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Max.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Max.cs index bf4a014a7b..f57d18d498 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Max.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Max.cs @@ -73,7 +73,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -177,7 +184,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -231,7 +245,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -285,7 +306,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -339,7 +367,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -393,7 +428,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Min.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Min.cs index 21bc0ccda5..b37cc2dfe9 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/Min.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/Min.cs @@ -78,7 +78,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -177,7 +184,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -231,7 +245,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -285,7 +306,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -339,7 +367,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -393,7 +428,14 @@ public override void OnCompleted() { if (!_hasValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleAsync.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleAsync.cs index 3d10d0bd96..52979456c5 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleAsync.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleAsync.cs @@ -33,7 +33,14 @@ public override void OnNext(TSource value) { if (_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT)); + try + { + throw new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT); + } + catch (Exception e) + { + ForwardOnError(e); + } return; } @@ -45,7 +52,14 @@ public override void OnCompleted() { if (!_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { @@ -101,7 +115,14 @@ public override void OnNext(TSource value) { if (_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_MATCHING_ELEMENT)); + try + { + throw new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_MATCHING_ELEMENT); + } + catch (Exception e) + { + ForwardOnError(e); + } return; } @@ -114,7 +135,14 @@ public override void OnCompleted() { if (!_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_MATCHING_ELEMENTS); + } + catch (Exception e) + { + ForwardOnError(e); + } } else { diff --git a/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleOrDefaultAsync.cs b/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleOrDefaultAsync.cs index 9bb3490e8e..c64c7d2487 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleOrDefaultAsync.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/Observable/SingleOrDefaultAsync.cs @@ -33,7 +33,14 @@ public override void OnNext(TSource value) { if (_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT)); + try + { + throw new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT); + } + catch (Exception e) + { + ForwardOnError(e); + } return; } @@ -94,7 +101,14 @@ public override void OnNext(TSource value) { if (_seenValue) { - ForwardOnError(new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_MATCHING_ELEMENT)); + try + { + throw new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_MATCHING_ELEMENT); + } + catch (Exception e) + { + ForwardOnError(e); + } return; } diff --git a/Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Blocking.cs b/Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Blocking.cs index 7223966dba..ebf83fbcce 100644 --- a/Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Blocking.cs +++ b/Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Blocking.cs @@ -235,6 +235,7 @@ private static TSource SingleOrDefaultInternal(IObservable sou { var value = default(TSource); var seenValue = false; + var moreThanOneElement = false; var ex = default(Exception); using (var evt = new WaitAndSetOnce()) @@ -247,7 +248,7 @@ private static TSource SingleOrDefaultInternal(IObservable sou { if (seenValue) { - ex = new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT); + moreThanOneElement = true; evt.Set(); } @@ -270,6 +271,11 @@ private static TSource SingleOrDefaultInternal(IObservable sou ex.ThrowIfNotNull(); + if (moreThanOneElement) + { + throw new InvalidOperationException(Strings_Linq.MORE_THAN_ONE_ELEMENT); + } + if (throwOnEmpty && !seenValue) { throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); diff --git a/Rx.NET/Source/src/System.Reactive/Threading/Tasks/TaskObservableExtensions.cs b/Rx.NET/Source/src/System.Reactive/Threading/Tasks/TaskObservableExtensions.cs index 7f629d6ddd..3b4ae1320f 100644 --- a/Rx.NET/Source/src/System.Reactive/Threading/Tasks/TaskObservableExtensions.cs +++ b/Rx.NET/Source/src/System.Reactive/Threading/Tasks/TaskObservableExtensions.cs @@ -470,7 +470,14 @@ public override void OnCompleted() } else { - _tcs.TrySetException(new InvalidOperationException(Strings_Linq.NO_ELEMENTS)); + try + { + throw new InvalidOperationException(Strings_Linq.NO_ELEMENTS); + } + catch (Exception e) + { + _tcs.TrySetException(e); + } } _ctr.Dispose(); // no null-check needed (struct) diff --git a/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/SingleAsyncTest.cs b/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/SingleAsyncTest.cs index b3af074363..c993a7f6d1 100644 --- a/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/SingleAsyncTest.cs +++ b/Rx.NET/Source/tests/Tests.System.Reactive/Tests/Linq/Observable/SingleAsyncTest.cs @@ -4,6 +4,9 @@ using System; using System.Reactive.Linq; +using System.Reactive.Threading.Tasks; +using System.Threading.Tasks; + using Microsoft.Reactive.Testing; using ReactiveTests.Dummies; using Xunit; @@ -253,5 +256,59 @@ public void SingleAsync_PredicateThrows() ); } + [Fact] // https://github.com/dotnet/reactive/issues/1235 + public void MeaningfulStackTrace() + { + static async Task Core() + { + static void AssertException(Exception e) + { + Assert.IsType(typeof(InvalidOperationException), e); + + Assert.NotNull(e.StackTrace); + Assert.NotEqual("", e.StackTrace); + + Assert.True(e.StackTrace.Contains("SingleAsync")); + } + + var xs = Observable.Range(0, 2).SingleAsync(); + + try + { + await xs; + } + catch (Exception e) + { + AssertException(e); + } + + try + { + await xs.ToTask(); + } + catch (Exception e) + { + AssertException(e); + } + + var tcs = new TaskCompletionSource(); + + xs.Subscribe( + _ => { }, + e => tcs.SetException(e), + () => tcs.SetResult(false)); + + try + { + await tcs.Task; + } + catch (Exception e) + { + AssertException(e); + } + } + + Core().GetAwaiter().GetResult(); + } } }