-
Notifications
You must be signed in to change notification settings - Fork 564
Improve JavaProxyThrowable by translating managed stack trace #8185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ee565e4
7c0a507
6ee31a3
7cfc3dc
78839fb
b2b38b1
ada140c
45b3e72
241ed9b
c1cabbc
4c38c11
7a8964b
1bac358
998c012
a25636d
92e5db3
4b5b56e
b68dcc2
32130ab
603f1e4
cdf4714
784014c
a78d466
691e075
af90f92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,83 @@ | ||
| using System; | ||
| using System.Diagnostics; | ||
| using System.Reflection; | ||
|
|
||
| using StackTraceElement = Java.Lang.StackTraceElement; | ||
|
|
||
| namespace Android.Runtime { | ||
|
|
||
| class JavaProxyThrowable : Java.Lang.Error { | ||
| sealed class JavaProxyThrowable : Java.Lang.Error { | ||
|
|
||
| public readonly Exception InnerException; | ||
|
|
||
| public JavaProxyThrowable (Exception innerException) | ||
| : base (GetDetailMessage (innerException)) | ||
| JavaProxyThrowable (string message, Exception innerException) | ||
| : base (message) | ||
| { | ||
| InnerException = innerException; | ||
| } | ||
|
|
||
| static string GetDetailMessage (Exception innerException) | ||
| public static JavaProxyThrowable Create (Exception innerException) | ||
| { | ||
| if (innerException == null) { | ||
| throw new ArgumentNullException (nameof (innerException)); | ||
| } | ||
|
|
||
| // We prepend managed exception type to message since Java will see `JavaProxyThrowable` instead. | ||
| var proxy = new JavaProxyThrowable ($"[{innerException.GetType ()}]: {innerException.Message}", innerException); | ||
|
|
||
| try { | ||
| proxy.TranslateStackTrace (); | ||
| } catch (Exception ex) { | ||
| // We shouldn't throw here, just try to do the best we can do | ||
| Console.WriteLine ($"JavaProxyThrowable: translation threw an exception: {ex}"); | ||
| proxy = new JavaProxyThrowable (innerException.ToString (), innerException); | ||
| } | ||
|
|
||
| return proxy; | ||
| } | ||
|
|
||
| void TranslateStackTrace () | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with this method is that it doesn't "merge", it "replaces". #1188 should have provided examples for desired behavior. Consider: var e = new Java.Lang.Error ("here!");
e.PrintStackTrace(Java.Lang.JavaSystem.Out);This produces the output: If the assumption in #1188 is correct, and we want As far as I can tell, as the complete stack trace, which I don't believe would be useful. (Now, this PR/#1188 is about |
||
| { | ||
| if (innerException == null) | ||
| throw new ArgumentNullException ("innerException"); | ||
| var trace = new StackTrace (InnerException, fNeedFileInfo: true); | ||
| if (trace.FrameCount <= 0) { | ||
| return; | ||
| } | ||
|
|
||
| StackTraceElement[]? javaTrace = null; | ||
| try { | ||
| javaTrace = GetStackTrace (); | ||
| } catch (Exception ex) { | ||
| // Report... | ||
| Console.WriteLine ($"JavaProxyThrowable: obtaining Java stack trace threw an exception: {ex}"); | ||
| // ..but ignore | ||
| } | ||
|
|
||
|
|
||
| StackFrame[] frames = trace.GetFrames (); | ||
| int nElements = frames.Length + (javaTrace?.Length ?? 0); | ||
| StackTraceElement[] elements = new StackTraceElement[nElements]; | ||
|
|
||
| for (int i = 0; i < frames.Length; i++) { | ||
| StackFrame managedFrame = frames[i]; | ||
| MethodBase? managedMethod = managedFrame.GetMethod (); | ||
|
|
||
| var throwableFrame = new StackTraceElement ( | ||
| declaringClass: managedMethod?.DeclaringType?.FullName, | ||
| methodName: managedMethod?.Name, | ||
| fileName: managedFrame?.GetFileName (), | ||
| lineNumber: managedFrame?.GetFileLineNumber () ?? -1 | ||
| ); | ||
|
|
||
| elements[i] = throwableFrame; | ||
| } | ||
|
|
||
| if (javaTrace != null) { | ||
| for (int i = frames.Length; i < nElements; i++) { | ||
| elements[i] = javaTrace[i - frames.Length]; | ||
| } | ||
| } | ||
|
|
||
| return innerException.ToString (); | ||
| SetStackTrace (elements); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have
Exception? innerExceptionif you're gonna throwArgumentNullExceptionhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a typo