xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.44k stars 507 forks source link

When used incorrectly `SCShareableContent.GetShareableContentAsync` produces a confusing SetupBlock NullReferenceException #15543

Open drasticactions opened 2 years ago

drasticactions commented 2 years ago

When you invoke "ScreenCaptureKit.SCShareableContent" an exception is thrown in the ObjCRuntime. From what I remember, whenever you run it for the first time in a given app, you need to ask for permission to use it, based on whatever method you use first. In this case, when I run it, it blows up.

 Failed WindowCaptureContext [< 1 ms]
  Error Message:
   Test method Drastic.ScreenRecorder.Mac.Tests.WindowTests.WindowCaptureContext threw exception: 
System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.) ---> System.NullReferenceException: Object reference not set to an instance of an object.
  Stack Trace:
      at ObjCRuntime.Runtime.ComputeSignature(MethodInfo method, Boolean isBlockSignature)
   at ObjCRuntime.BlockLiteral.SetupBlock(Delegate trampoline, Delegate userDelegate, Boolean safe)
   at ObjCRuntime.BlockLiteral.SetupBlockUnsafe(Delegate trampoline, Delegate userDelegate)
   at ScreenCaptureKit.SCShareableContent.GetShareableContent(Boolean excludeDesktopWindows, Boolean onScreenWindowsOnly, Action`2 completionHandler)
   at ScreenCaptureKit.SCShareableContent.GetShareableContentAsync(Boolean excludeDesktopWindows, Boolean onScreenWindowsOnly)
   at Drastic.ScreenRecorder.Mac.WindowEnumeration.GetWindowsAsync() in /Users/drasticactions/Developer/Personal/Drastic.ScreenRecorder/src/Drastic.ScreenRecorder.Mac/WindowEnumeration.cs:line 18
--- End of inner exception stack trace ---
    at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at System.Threading.Tasks.Task`1.get_Result()
   at Drastic.ScreenRecorder.Mac.WindowEnumeration.GetWindows() in /Users/drasticactions/Developer/Personal/Drastic.ScreenRecorder/src/Drastic.ScreenRecorder.Mac/WindowEnumeration.cs:line 12
   at Drastic.ScreenRecorder.Tests.SharedTests.WindowEnumeration(IWindowEnumeration windowEnumeration) in /Users/drasticactions/Developer/Personal/Drastic.ScreenRecorder/src/Drastic.ScreenRecorder.Tests/SharedTests.cs:line 38
   at Drastic.ScreenRecorder.Mac.Tests.WindowTests.WindowCaptureContext() in /Users/drasticactions/Developer/Personal/Drastic.ScreenRecorder/src/Drastic.ScreenRecorder.Mac.Tests/WindowTests.cs:line 23

Steps to Reproduce

You can reproduce the error by running these tests: https://github.com/drasticactions/Drastic.ScreenRecorder/tree/main/src/Drastic.ScreenRecorder.Mac.Tests

The code is https://github.com/drasticactions/Drastic.ScreenRecorder/blob/main/src/Drastic.ScreenRecorder.Mac/MonitorEnumeration.cs#L15-L22

Expected Behavior

The method would either show a prompt on the screen asking for permission, or it would return the list of Displays or windows.

Actual Behavior

Null Reference Exception.

Environment

macos 12.4 net6 macOS SDK 12.3.440

Version information ``` ```

Build Logs

Example Project (If Possible)

drasticactions commented 2 years ago

Scratch that, I think I got it figured out.

It is because of the test context. I had my app logic screwed up, but once I set up the NSRunLoop it fired without a crash. But that exception is pretty ugly. If you didn't know you needed to do that, you would be kinda stuck.

If you setup a console app with

using ScreenCaptureKit;

NSApplication.Init();

SCShareableContent.GetShareableContent(false, false, (arg1, arg2) => { 
Console.WriteLine("It works");
});

NSRunLoop.Current.RunUntil(NSDate.DistantFuture);

It will fire correctly and not NRE. But if you don't init the application or run the run loop, you'll get the NRE. IMO, if there's a way to make that more clean as an exception that you need to setup that context, that would be good.

chamons commented 2 years ago

When @rolfbjarne is back in the office, he can take a look and see if it possible to improve the exception here (since it's deep dark block code).

If so, great, I do agree it's a confusing exception, else we'll just close it as "best we can do" otherwise. Sometime Apple's APIs do not return anything but nil when you mess up, and it's confusing.

rolfbjarne commented 2 years ago

We can certainly do better than a NRE, but I'm not sure it would be much more helpful in this particular scenario (should still be worth fixing though).