beeware / briefcase

Tools to support converting a Python project into a standalone native application.
https://briefcase.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
2.62k stars 366 forks source link

Suggession: Add error capture flag to "run" command #314

Closed saroad2 closed 2 years ago

saroad2 commented 4 years ago

Hey, How are you doing? :)

I have a problem where my program is working just fine when I run it with briefcase dev but fails on briefcase run. Since the run command doesn't capture errors, I cannot see the reason for the failure.

As I've seen the run command starts a process using the subprocess command. To help developers debug problems like this I suggest add a flag capture-errors that run the subprocess in this following way (example from msi.py):

        error_io = io.StringIO()
        try:
            print()
            self.subprocess.run(
                [
                    str(self.binary_path(app) / 'src' / 'python' / 'pythonw.exe'),
                    "-m", app.module_name
                ],
                check=True,
                stderr=error_io
            )
        except subprocess.CalledProcessError:
            print(error_io.getvalue())
            raise BriefcaseCommandError(
                "Unable to start app {app.app_name}.".format(app=app)
            )

In that way, the errors will be printed and we could see the problem. In the absence of this flag, it will run it without the error_io

What do you think about the solution? Once you'll approve it, I'll implement it.

paulproteus commented 4 years ago

Hi @saroad2 ! I ran into a similar problem recently, and I have a suggestion. @freakboy3742 made most of the design decisions in briefcase and may have a different take; whatever he says is fine with me, too.

My suggestion is that when briefcase starts any subprocess, it always can store a debug log in ~/.briefcase. If the command fails, briefcase prints the path to that debug log. If the subprocess succeeded, briefcase can delete the log file. This avoids the possibility of memory bloat, though it introduces the possibility that the command will print so much output as to fill up the disk, but I think that is less likely than filling up RAM.

I like this because it would have helped me debug my Android changes recently.

We could make the debug log filename could be different for each execution, or we could reuse the same filename over and over; I have no real opinion on this.

I suggest we write a helper function around subprocess that implements this logic, rather than directly calling subprocess.run(). An initial implementation could adjust just one or two places to use this new mechanism, to see if we really do like it.

saroad2 commented 4 years ago

I generally like the idea of saving the error log into a file, but I don't think that this should happen always. I think it should be configurable from the cmd as a flag or something.

You don't want to save the logs always. you want to do it in case it's needed (such as your case and mine). In that way, you won't blow up the RAM.

wrapping self.subprocess in a class is an excellent idea that I really like. Whenever @freakboy3742 will comment on the ideas and choose the desired approach, I'll go on implementing the solution.

freakboy3742 commented 4 years ago

Agreed that this is a problem that needs solving. You're seeing it manifest on Windows, but there's an often-reported problem on macOS that is effectively the same thing - "when I run the packaged app, and it crashes, I get no console feedback". On macOS it's particularly bad because the error that does get reported is a very helpful LSOpenURLsWithRole() failed with error -10810, which means absolutely nothing to most users.

I generally agree with @paulproteus that the solution should be logging based; however, I'd argue that it should be in a platform-appropriate location for apps, rather than a briefcase-specific one. I fail to see how RAM is a constraining factor; disk space maybe if you're especially noisy in your logs - but that's what log rotation is for.

It may also be appropriate, for diagnostic purposes, for briefcase to pipe the current logfile when running briefcase run.

saroad2 commented 4 years ago

So where do you want to locate the log file? I don't think I know what you mean by "platform-appropriate location for apps". Can you elaborate where to save the log file for each platform?

freakboy3742 commented 4 years ago

I can't give the locations for every platform off the top of my head - I'd need to do some research. However, I can tell you that macOS requires that sandboxed app content goes in ~/Library/Application Support; it also recommends using the console log (which is a system service for logging). That is where logs for a macOS app should go. Android and iOS both have their own logging services - that is where their logs should go.

What I'm trying to convey is that we shouldn't pick a location at random, or a location that seems right to us, on one platform - on each platform, we should pick the location that is appropriate (and UX compliant) for the platform.

saroad2 commented 4 years ago

I'll start implementing it for windows and we'll go on from there. As I've checked the best place for logs in the Windows platform is "HOMEDIR\AppData\Local\Temp\PROGRAM\", which in our case PROGRAM is "briefcase". Got the answer from here.

Sounds good?

freakboy3742 commented 4 years ago

That location makes sense; I'd probably append "Logs" to the end to avoid any possible collision with other application data.

Thinking about this a bit more: While logging directly from the run command is an immediate workaround, there's a bigger issue that needs to be solved. The task of the run command should be "start the app and tail the logs" - run shouldn't be managing logging itself at all.

The Windows run implementation is currently a bit of a hack to work around the fact that the windows "app" isn't really an app at all, but an invocation of pythonw linked from the Start menu. What is really needed is a genuine "executable" entry point that can have an icon, manage the configuration of logging, and so on.

One last thing: the approach you've described in your patch won't be sufficient. That approach won't display any output until the application stops. Console output needs to be streamed, not dumped in a big chunk at the end of processing (and it needs to capture both stdout and stderr, not just stderr).

saroad2 commented 4 years ago

We can try use the py2exe package to make it an executable. I must say that before I looked under the hood I thought that this is what briefcase build does. Maybe I can try implement it, seeing how hard it is to do.

freakboy3742 commented 4 years ago

No - Py2exe isn't an option. The approach used by py2exe is fragile under many circumstances, and is fundamentally incompatible with how Briefcase is very deliberately trying to build it's applications.

saroad2 commented 4 years ago

I see. so, for now, I'll just continue with adding the logs option in windows. If you have an idea of how to replace pythonw with *.exe file, let me know.

saroad2 commented 4 years ago

How about using this to package the application? https://pyinstaller.readthedocs.io/en/stable/

freakboy3742 commented 4 years ago

Not really; Pyinstaller is effectively a "competitor" - it's another entire packaging tool with it's own packaging story.

However, it may be interesting to investigate how Pyinstaller builds it's executable entry points on Windows. That one small piece of PyInstaller might be useful to re-use/adapt, depending on how they're doing it.

freakboy3742 commented 4 years ago

Although - on closer inspection, PyInstaller code is GPL, which means using it in any way, even as inspiration for an approach in Briefcase, would be legally problematic. I'd recommend avoiding looking at the code if you're planning to continue contributing to Briefcase.

BrendanSimon commented 4 years ago

This may be useful ?

https://www.tillett.info/2013/05/13/how-to-create-a-windows-program-that-works-as-both-as-a-gui-and-console-application/

BrendanSimon commented 4 years ago

I had a look at the Windows shortcut. It has a target to the install directory, which calls pythonw.exe. Console apps need to be invoked with python.exe.

I changed the target to use python.exe and a console/terminal flashes up (I can see my output briefly), then the console/terminal disappears.

I copied the target to a batch file and added a %* to the end. Now I can call the batch file (with arguments) from within a console and it works as expected. If the batch file is in the path then it can be called from anywhere (from any directory).

This doesn't help briefcase run per se, but that part of briefcase could be enhanced to call pythonw.exe or python.exe depending on a setting in the pyproject.toml file (and for windows only).

briefcase new prompts user for a GUI toolkit (Toga, PySide2 or None). This could be enhanced to also create a setting in the project file to specify the user interface (essentially GUI or Console). This may only be applicable for Windows and or macOS ?

e.g. ui='Toga' or ui='PySide2' or ui=None or ui='console' ?

NOTE: I tried PyInstaller and it has the --windowed/--nowindow option which builds the resultant exe as either a gui or console app.

Also PyInstaller generates a .exe file with the PE header (for both --onefile and --onedir options) This is different to the output of briefcase, which generates the equivalent of --onedir but doesn't generate a .exe. Briefcase, via WiX, installs a windows shortcut file (.lnk) in the start menu, which has a target that explicitly calls the bundled pythonw.exe with the module name that is bundled. e.g. pythonw.exe -m myapp

freakboy3742 commented 2 years ago

This has been addressed by #591, released in Briefcase 3.6 - we now capture logs for running apps; the only exception is on Windows, which will be addressed by #620.