Throw error on null bitmap to prevent crash

[lewisl9029]

I've been exploring using nut.js (and more recently the libnut library directly) to control a headless Windows machine monitored through RDP (Using the official Remote Desktop app).

The setup generally worked fine, but I noticed the node process ends up consistently crashing as soon as I disconnect the RDP session.

Dug into it a bit this weekend and found that the crash was due to this null returning branch in copyMMBitmapFromDisplayInRect being unhandled downstream.

This PR handles the null case by throwing an exception so it can be handled in userland appropriately (in my case a simple retry after some delay) instead of crashing the entire program.

It's been a while since I wrote any C, so please let me know if there's any other cleanup steps necessary before throwing the exception that I might have missed. Thanks!

[s1hofmann]

Hi @lewisl9029 👋

Thanks for your contribution!
You're right, this should definitely be handled in userland.

Just two remarks:

1. Would you mind creating a corresponding issue for this PR? I'd like to link to issues instead of merged PRs for the changelog
2. Is there a way to properly test this change on all platforms? I'd would love having a test for every PR, so maybe you want to take another look at it (maybe even join our dev channels on Discord?). If you don't find time for it, no problem. I'll see for myself then.

[lewisl9029]

Hi there!

Here's the issue I've created, also listed out the repro steps: #55

Re: testing, I'd love to be able to write a test for this, but we'd either have to repro the conditions that led to the null branch in copyMMBitmapFromDisplayInRect in a CI environment somehow, which I have no idea how to do since it involves connecting/disconnecting from RDP, or mock the function in C somehow, when the existing tests seem to be focused on testing the public interface in JS and doesn't appear to have that ability.

Would love to hear if you have some suggestions on how we could approach testing this though.

[s1hofmann]

One approach to test this on Linux would be to run a test without Xvfb, but I don’t have approaches for Windows and macOS. Making the cross-platform screen grabbing testable would require a little restructuring, which I think doesn’t have to happen here.

Let‘s deal with improving testability of screen grabbing in a separate issue.