Allows Debug.Fail to go through Trace Listeners #20764
Conversation
maryamariyan
force-pushed
the
debug-fail
branch
from
3804f86 to
ec3b730
Compare
maryamariyan
changed the title
maryamariyan
force-pushed
the
debug-fail
branch
from
ec3b730 to
ec028d6
Compare
| { | ||
| if (s_ShowDialog != null) | ||
| { | ||
| s_ShowDialog(stackTrace, message, detailMessage, errorSource); |
There was a problem hiding this comment.
If this can be set to null, there's a potential race condition here, and s_showDialog should be stored into a local first and then that local checked and used.
There was a problem hiding this comment.
s_ShowDialog is a hook for our existing internal tests only.
src/System.Private.CoreLib/shared/System/Diagnostics/DebugProvider.Unix.cs
Outdated
Show resolved
Hide resolved
| string stackTrace; | ||
| try | ||
| { | ||
| stackTrace = new StackTrace(0, true).ToString(System.Diagnostics.StackTrace.TraceFormat.Normal); |
There was a problem hiding this comment.
Do the extra stackframes between the user code and this place show up in the stacktrace now?
There was a problem hiding this comment.
No they don't. Only shows up until the user code, the line that calls Debug.Fail or Trace.Fail.
That's what we want right?
|
@dotnet-bot test CentOS7.1 x64 Debug Innerloop Build |
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
I think this completely overlooks and ultimately breaks the developer experience - irrespective of a stack (web or desktop, Windows or not) a developer should be presented with an assert popup, if it gets hit. Right now the debug experience for .NET Core/.NET is totally broken, an app just crashes, if it is launched outside a debugger. A VS experience isn't better either: VS Code is offering a better DevEx, but it can't be used to develop desktop apps: |
|
I interpreted the comment to mean a new API should not be necessary in order to get Debug.Assert to work, not that it wasn't important to get it to work. |
|
@RussKie Thank you for bringing it up. I have opened dotnet/runtime#40793 |
5 participants
Note:
@jkotas:
Arguably, as mentioned by you earlier in https://github.com/dotnet/corefx/issues/33110 and as illustrated here, Trace Listeners can be used to show a custom dialog.
As a result, I couldn't find enough reason for adding any public API for ShowDialog.
This PR helps link Debug.Assert and Debug.Fail with Trace Listeners.
corefx PR: dotnet/corefx#33212
cc: @jkotas @danmosemsft @eerhardt
UPDATE