cli-microsoft365: Bug report: m365 cli reconsent seems not to open the browser automatically when autoOpenLinksInBrowser is true

As I need this to fix #4037 , I would love to pick this one up.

After some research from me and @milanholemans we noticed that when we added a delay after calling open('https://google.be') by for example simply adding a for loop that would iterate 100.000 items, the browser would open after approximately 3 seconds. So it seems that the method open is not properly awaited by the module that we are using. I have logged an issue at the repository which can be found at the following url: https://github.com/sindresorhus/open/issues/298

Adding the option wait: true is also not an option to resolve this issue, as it will then wait for macbook users until the user has explicitly closed the entire application, which would be very annoying. I’m wondering if this functionality has worked in the past. Are there any old Windows-users who maybe have experience using this and remembering that it has indeed worked?

Thank you for your opinions. Let’s use that approach. Let’s create a util function to open links in the browser using this approach.

Ready to open it up!

Can confirm that this works properly 😄

@milanholemans asked me to verify on OS X. Will do this tomorrow!

Gotcha. If that’s needed for win, then let’s use it and also pay attention when we update open if the behavior is still correct.

Indeed, because I think this behavior is not really intentional.

I suggest that we create a util function to open URLs in browsers so we don’t have to do the OS check in all these different commands.

Due to the lack of response on the open repo, I’ve been experimenting with it and might be onto something. The open command returns a process object which contains the process ID (pid). I found out (at least on Windows) that the process is being started, but we should wait for it to terminate before executing the rest of the code. If we don’t wait for it to terminate, the browser will not open.

There are 2 solutions, either we take the pid and poll every few milliseconds if the process is still active. When the process doesn’t exist anymore, we can execute the rest of our code.

In pseudo code:

const process = await open(url, { wait: false });

let processActive = true;
while (processActive) {
  sleep(100);
  processActive = await processExists(process.pid);
}

// Rest of the code

But a better way to me, in the process object, we can add listeners to this process. This enables us to add a listener when the process terminates.

In code this looks like this:

private async openBrowser(link: string): Promise<void> {
  return new Promise<void>(async (res) => {
    const process = await open(link, { wait: false });
    process.addListener('exit', () => res());
  });
}

Tried this function locally and seems to be working for me 100% of the time. The only question is, will this also work for Mac and other OSs? My guess is that it probably won’t work there. But worst case we can perform a check on which OS we are before executing this piece of code.

While writing this I realize it could be easier to just do something like the code below which will do probably the same as my code.

const runningOnWindows = process.platform === "win32"

await open(url, { wait: runningOnWindows });

You are on a Mac though, right? Any other windows users who can check this? @pnp/cli-for-microsoft-365-maintainers?

Yes! I can confirm it doesn’t open my browser.

Me as well 👍

I’m also more of a fan of checking win32 platform

I vote for checking for win32. Looks simpler, and easier to revert when it’s no longer needed.

@milanholemans awesome find 👍. All in for the util method 👍 I think it seems like a solid plan and we could open it up right?

Gotcha. If that’s needed for win, then let’s use it and also pay attention when we update open if the behavior is still correct.

Does it take 5 or 6 for the window to show up or does our code need that time or the browser won’t launch? I’m not worried if it’s the former but if it’s the latter then it would be odd indeed.

That’s hard to say. My guess is that we really have to wait and don’t resolve our command promise until the window opens.

Ah, that certainly makes sense. 🤙

I just noticed the same behavior for command m365 cli issue --type command . Also using Windows though. Wanted to reuse this functionallity for #4037 , but can’t seem to get it working. When placing a try catch around it, it will also not go into the catch statement. Have also tried both awaiting and not awaiting, but nothing changes. Also specified the browser to use using await open('https://pnp.github.io/cli-microsoft365/', {app: {name: 'firefox'}}); for example, but without success.

On first sight, this doesn’t seem to fix the issue. Haven’t digged deeper into it yet.

Ok, too bad… more research necessary in that case…

No @waldekmastykarz, I mean my dev env is running in docker. I do use the CLI in PowerShell in windows.

To run a build and test it in windows I’d have to clone and build it in windows.

I knew @milanholemans doesn’t work in docker, that’s why I asked him

You are on a Mac though, right? Any other windows users who can check this? @pnp/cli-for-microsoft-365-maintainers?