Skip to content
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

Better error handling for capturing as an image #2673

Open
rmartin16 opened this issue Jun 22, 2024 · 2 comments
Open

Better error handling for capturing as an image #2673

rmartin16 opened this issue Jun 22, 2024 · 2 comments
Labels
bug A crash or error in behavior.

Comments

@rmartin16
Copy link
Member

rmartin16 commented Jun 22, 2024

Describe the bug

Failing to capture a screenshot can result in unexpected failure modes.

For instance, on Linux under Wayland, it is not possible to screenshot the entire screen:

[helloworld] Starting app...
===========================================================================

(helloworld:3383114): dbind-WARNING **: 11:52:50.660: Couldn't connect to accessibility bus: Failed to connect to socket /run/user/1000/at-spi/bus_1: Connection refused
window1.as_image()=<toga.images.Image object at 0x7f809e3fa320>
/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/__init__.py:57: NotImplementedWarning: [GTK] Not implemented: Screen.get_image_data() on Wayland
  warnings.warn(NotImplementedWarning(f"[{platform}] Not implemented: {feature}"))
Traceback (most recent call last):
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga_gtk/app.py", line 67, in gtk_startup
    self.interface._startup()
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/app.py", line 641, in _startup
    self.startup()
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app/helloworld/app.py", line 37, in startup
    print(f"{self.screens[0].as_image()=}")
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/screens.py", line 43, in as_image
    return Image(self._impl.get_image_data()).as_format(format)
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/images.py", line 155, in __init__
    raise TypeError("Unsupported source type for Image")
TypeError: Unsupported source type for Image

Instead, you get a runtime NotImplementedWarning (that's also part of a realllly long line of text) followed by a TypeError. This is because the screenshot API expects the backend to always return valid image data; in this case, None is returned.

This is also true of toga.widgets.Canvas.as_image() and toga.window.Window.as_image().

In a slightly different way, toga.widgets.ImageView.as_image() is also affected.

[helloworld] Starting app...
===========================================================================

(helloworld:3401633): dbind-WARNING **: 12:05:35.097: Couldn't connect to accessibility bus: Failed to connect to socket /run/user/1000/at-spi/bus_1: Connection refused
window1.as_image()=<toga.images.Image object at 0x7135f1c6f730>
self.screens[0].as_image()=<toga.images.Image object at 0x7135f1c6fe80>
Traceback (most recent call last):
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga_gtk/app.py", line 67, in gtk_startup
    self.interface._startup()
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/app.py", line 641, in _startup
    self.startup()
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app/helloworld/app.py", line 39, in startup
    print(f"{toga.ImageView().as_image()=}")
  File "/home/russell/tmp/beeware/helloworld/build/helloworld/pop/jammy/helloworld-0.0.1/usr/lib/helloworld/app_packages/toga/widgets/imageview.py", line 139, in as_image
    return self.image.as_format(format)
AttributeError: 'NoneType' object has no attribute 'as_format'

Here, users can create an empty ImageView and end up with an AttributeError if it doesn't actually contain an image.

Steps to reproduce

Sample app:

import toga

class HelloWorld(toga.App):
    def startup(self):
        self.main_window = toga.MainWindow(title=self.formal_name)
        self.main_window.show()

        print(f"{self.main_window.as_image()=}")
        print(f"{self.screens[0].as_image()=}")
        print(f"{toga.ImageView().as_image()=}")

def main():
    return HelloWorld(

Expected behavior

While some of these failure modes are user induced, others are quite subtle and may be entirely unknown to developers at build time. Toga could be more resilient to these failure modes and provide better error handling when they occur.

Screenshots

No response

Environment

  • Operating System: pop os 22.04
  • Python version: 3.10.14
  • Software versions:
    • Briefcase: 0.3.19
    • Toga: 0.4.6.dev52+g10608425a

Logs

No response

Additional context

On a side note, there are also likely to be interesting issues if you use these APIs while the window is not being shown. I think this may be getting a little in to particularly contrived failure modes, though.

@rmartin16 rmartin16 added the bug A crash or error in behavior. label Jun 22, 2024
@freakboy3742
Copy link
Member

The question for me is whether it's better to raise an error, or succeed, but with a dummy image and a log message. Raising an error is likely an "app exiting" behavior; but returning a screenshot that is the right size, but empty, with a clear log message that "screenshots aren't supported on Wayland" would at least allow an app to continue, and would require less safety checks (and typing updates) to ensure that None is a valid input throughout the system.

@mhsmith
Copy link
Member

mhsmith commented Jun 23, 2024

This kind of error is likely to happen to end users who are running in an environment that the app developer hasn't tested. In that case, an exception is better, because it would bring up a crash dialog with the message. If we used a log message instead, it would be invisible to the user, so the app developer could only guess at the cause.

Allowing the app to continue probably isn't useful in this case, because it can't do anything useful with an empty screenshot. If the app has other functions, then the user can just avoid the crash by not using the screenshot feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior.
Projects
None yet
Development

No branches or pull requests

3 participants