[Question] interactive_sample.py hangs indefinitely if you close the browser without doing the login

[psimoes123]

Describe the bug
I noticed that when you don't set up a timeout for the request and you close the browser without doing the login. The thread that gets created for the request doesn't terminate, which cause the application to hang indefinitely.

To Reproduce
Steps to reproduce the behavior:

1. interactive_sample.py
2. pretty much just follow the workflow there.

Expected behavior
I've used the msal library in other languages and the expected behavior is usually a thrown exception when the browser is closed.

What you see instead

a hanging thread that runs indefinitely. In the screenshot, I've paused the thread to see the stack trace.

The MSAL Python version you are using
1.32.0

Additional context
In the file authcode.py AuthCodeReceiver._get_auth_response method. the method self._server.handler_request() is being called and is basically waiting a request, which I assume is waiting a response for the login browser. When the browser gets closed before the login, no message is sent to the _server, which basically is in a infinite loop waiting for a request (it doesn't have timeout). if you are using a terminal you need to ctrl+c to stop the running script and if you are using a gui like Tkinter or something, you basically need to force quit the application.

[rayluo]

I noticed that when you don't set up a timeout for the request and you close the browser without doing the login. The thread that gets created for the request doesn't terminate, which cause the application to hang indefinitely.

I believed that it was you who didn't set up a timeout for the request. :-) Our sample tend to not list all parameters. You shall look into our docs.

[psimoes123]

I noticed that when you don't set up a timeout for the request and you close the browser without doing the login. The thread that gets created for the request doesn't terminate, which cause the application to hang indefinitely.

I believed that it was you who didn't set up a timeout for the request. :-) Our sample tend to not list all parameters. You shall look into our docs.

I understand that. I saw that timeout is an optional parameter and such. What I find hard to understand is that the socket server can wait indefinitely or until the timeout when the edge case scenario (a user closes the browser without completing the sign-in). This is due to being completely dependented on the timeout option. Additionally, if you look at other version of this library like msal for .net if the browser is closed prematurely, it will raise an exception immediately, meaning that it doesn't need a timeout to unblock the user, so they can retry again. Having to force quit or interrupt the script is not ideal. What I suggest is that when opening the webbrowser, we should be able to acquire a pid for that browser. Then keep track if the process still exist or if it was closed. In this manner, if the browser is closed, you can immediately raise an exception and the user is free to reopen again. I apologize if I didn't explain correctly in my initial post (this is my first time doing a bug report). 😁 You could say that this is not a bug per se, but rather an enhancement to the original implementation that will increase usability.

[rayluo]

other version of this library like msal for .net if the browser is closed prematurely, it will raise an exception immediately, meaning that it doesn't need a timeout to unblock the user

What UI did you see there? If it was a smaller pop-up window, it is a webview which provides finer control for library maintainers. But in Python, we can only open a new tab/window of a real browser, and we don't even get a pid of that browser (it might even just be a thread within the browser process). If there is a better way, please let us know.

Meanwhile, the current implementation, even with its timeout=None default behavior, works well enough for scripts which stays on the front ground anyway, such as az login, because users can always do a CTRL+C to abort. We are well aware of that. So, no, the current behavior is definitely not a bug; we welcome feasible enhancement proposal, for sure. What kind of app are you building? Is it not a script?

All that being said, nowadays there is also another option, which is to use a broker. Broker handles its own UI, so, it is nicer for UX.

[psimoes123]

What UI did you see there? If it was a smaller pop-up window, it is a webview which provides finer control for library maintainers. But in Python, we can only open a new tab/window of a real browser, and we don't even get a pid of that browser (it might even just be a thread within the browser process). If there is a better way, please let us know.

I've created projects with winforms, wpf, and maui (they were samples to test msal functionalities). Also, I've used all the options they have like regular web browser, embedded webbrowser (which defaults to webview2 and if not available then default to a in-build web browser control depending on the UI framework), and also I've used broker which defaults to webview2. I believe all have similar behavior. When the browser is closed prematurity, it throws an exception.

Yes, you are correct the webbrowser module in python does not provide directly the pid. But internally it is using subprocess.popen() which has access to the pid. So it means that we would need to find a different library or create a custom library that provides that functionality.

Meanwhile, the current implementation, even with its timeout=None default behavior, works well enough for scripts which stays on the front ground anyway, such as az login, because users can always do a CTRL+C to abort. We are well aware of that. So, no, the current behavior is definitely not a bug; we welcome feasible enhancement proposal, for sure. What kind of app are you building? Is it not a script?

I'm building a desktop app using tkinter (I'm no expert in tkinter), which is also a sample project to show the msal functionality. I think when I use az login, at least in my machine is using the embedded browser (broker), so when it is closed, It automatically abort the script. Yes, my application has optionality for the broker in a config file. I didn't get around to test it yet.

[rayluo]

I'm building a desktop app using tkinter

Yeah, try and see whether broker is what you want.

In the traditional browser code path, I think we could possibly add a hacky callback for app developer to break the waiting loop, however, as mentioned above, there is still no feasible way to auto-detect the closure of a browser or one of its tab; that means your GUI app would still need to somehow link a "Cancel" button to the currently nonexistent callback. That does not sound good enough for you, so, we won't bother; just go with the broker solution.

[psimoes123]

@rayluo , I was thinking along the lines of just returning subprocess itself back in the webbrowser module when calling webbrowser.new_open() [altering the return type from boolean to being the subprocess itself] and then passing the subprocess to the server as an optional parameter so that it can call subprocess.poll() every couple seconds to check if the browser is still opened. That way you are able to auto-detect when the new web browser is closed. Don't you think this is a reasonable and feasible solution that would give a better user experience?
Also, the sample application I'm creating is to allow the user to test different optionality. Using the broker or not, using cache and things of that nature. So I can't just say, use only broker when you are doing python.
I would really appreciate if it would at least be taken as a possible enhancement for the future.

[rayluo]

thinking along the lines of just returning subprocess itself back in the webbrowser module when calling webbrowser.new_open() (altering the return type from boolean to being the subprocess itself) ...

Not sure whether you realized that the webbrowser.open() is part of Python's standard library. Unless that kind of new behavior would be provided by Python out of the box, we as a downstream library don't usually consider maintaining our own fork.

... and then passing the subprocess to the server as an optional parameter so that it can call subprocess.poll() every couple seconds to check if the browser is still opened.

Even if we would do that, I still don't think you/we can assume a new subprocess will be created exclusively for our login page. Browser could very likely use one of its subprocess to serve many tabs, therefore closing the login tab will not result in the disappearing of the subprocess.

All in all, feel free to explore and share some code snippet of any promising progress. This is an opensource library anyway.