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

[bug] deno-webui crashes if you open a window after closing all windows #66

Open
DevRubicate opened this issue Dec 13, 2024 · 24 comments
Open
Labels
bug Something isn't working

Comments

@DevRubicate
Copy link
Contributor

DevRubicate commented Dec 13, 2024

If at any point you close all windows spawned by deno-webui, you will be unable to spawn any new windows as deno will crash from an uncaught error:

error: Uncaught (in promise) Error: unable to start the browser
      throw new WebUIError(`unable to start the browser`);
            ^
    at WebUI.show (https://deno.land/x/webui@2.5.0/src/webui.ts:105:13)
    at file:///.../deno-webui/examples/hello_world/hello_world.ts:125:10
    at eventLoopTick (ext:core/01_core.js:214:9)

To see this bug in action, simply modify the hello_world.ts example so that it works like this:

// Show the window
myWindow.show(myHtml); // Or myWindow.show('./myFile.html');

// Wait until all windows get closed
await WebUI.wait();

console.log("Thank you.");

// Show the window again
myWindow.show(myHtml); // Crash here!

// Wait until all windows get closed again
await WebUI.wait();

This happens to me on Windows 11.

@AlbertShown
Copy link
Contributor

Thank you for sharing 👍
I'm working on it...

@AlbertShown AlbertShown added the bug Something isn't working label Dec 13, 2024
@AlbertShown
Copy link
Contributor

Clone c703d73

async function allEvents(e: WebUI.Event) {
  /*
    e.window: WebUI;
    e.eventType: number;
    e.element: string;
  */
  console.log(`allEvents: window = ${e.window}`);
  console.log(`allEvents: eventType = ${e.eventType}`);
  console.log(`allEvents: element = ${e.element}`);
  if (e.eventType == 0) {
    console.log(`Window closed, open it again...`);
    myWindow.show(myHtml);
  }
}
WebUI.setTimeout(0);
WebUI.setMultiClient(true);
myWindow.bind("", allEvents);

// Show the window
myWindow.show(myHtml); // Or myWindow.show('./myFile.html');

// Wait until all windows get closed
await WebUI.wait();

console.log("Thank you.");

@DevRubicate
Copy link
Contributor Author

Success!

Jumping to that commit id and testing with your code appended to the hello_world.ts example I get new windows after closing the previous one and no errors!

Thank you for the incredibly fast response. 👌

@DevRubicate
Copy link
Contributor Author

Oh, but you typed setMultiClient's parameter as Boolean, you problaby want to type it as boolean

@AlbertShown
Copy link
Contributor

You are right, would you please fix it in a PR?
Otherwise, I will update it later this week...

@DevRubicate
Copy link
Contributor Author

No problem, I've created a pull request.

@DevRubicate
Copy link
Contributor Author

DevRubicate commented Dec 18, 2024

Huh... it appears that by using WebUI.setMultiClient(true) then the script() method of WebUI instances stops working, and in a very odd way, it simply rejects() it's promise, and the reject value is an empty string, no proper error message.

When I don't use multiclient, .script(`return JSON.stringify('test');`) works, and when I do use multiclient then the same code fails.

@AlbertShown
Copy link
Contributor

AlbertShown commented Dec 18, 2024

Good catch!
I forgot to tell you that when using default single client mode, we use webui_script()
but when we use multi-clients mode, then we should use webui_script_client() instead!

Ref: https://webui.me/docs/2.5/#/?id=script.

@AlbertShown
Copy link
Contributor

AlbertShown commented Dec 18, 2024

Okay. To make this work, we need to add .scriptClient() to Deno wrapper.
The new API .scriptClient() will simply call the core API webui_script_client().
But because webui_script_client() needs webui_event_t* e as a parameter, and Deno does not have this, then Deno should call webui_interface_script_client() instead, because it does not need webui_event_t* e... but those new interfaces are not yet implemented.

However, I created this issue, hopefully those interfaces will be implemented soon... they are easy to implement anyway...

@DevRubicate
Copy link
Contributor Author

I understand, thank you for the explanation. Meanwhile I can live without setMultiClient(true) while developing 🙂

@AlbertShown
Copy link
Contributor

AlbertShown commented Dec 20, 2024

scriptClient is available now (e3511bd).

async function myCallback(e: WebUI.Event) {
    e.window.scriptClient(e, `alert('test');`);
}

setMultiClient(true);

@DevRubicate
Copy link
Contributor Author

Thank you,

However, something appears to have gone wrong. The example you gave above showing WebUI.setMultiClient(true); no longer appears to work with this Deno-WebUI v2.5.1 release. When I try to run it I am back to getting this error when opening the second window:

C:\...\deno-webui\examples\hello_world>deno --allow-all hello_world.ts
allEvents: window = [object Object]
allEvents: eventType = 1
allEvents: element =
allEvents: window = [object Object]
allEvents: eventType = 0
allEvents: element =
Window closed, open it again...
error: Uncaught (in promise) Error: unable to start the browser
      throw new WebUIError(`unable to start the browser`);
            ^
    at WebUI.show (https://deno.land/x/webui@2.5.1/src/webui.ts:100:13)
    at allEvents (file:///D:/sync/repo/deno-webui/examples/hello_world/hello_world.ts:36:14)
    at https://deno.land/x/webui@2.5.1/src/webui.ts:406:39

@AlbertShown
Copy link
Contributor

I just re-test it, it's working fine!
Please make sure you are using webui@2.5.1
And make sure the cache Deno cache folder is ... ... ...\x\webui@2.5.1\src\webui-xxx-xxx-x64

@AlbertShown
Copy link
Contributor

import { WebUI } from "https://deno.land/x/webui@2.5.1/mod.ts";

const myHtml = `<html><script src="webui.js"></script> Testing Re-Open window! </html>`;

const myWindow = new WebUI();

async function allEvents(e: WebUI.Event) {
  /*
    e.window: WebUI;
    e.eventType: number;
    e.element: string;
  */
  console.log(`allEvents: window = ${e.window}`);
  console.log(`allEvents: eventType = ${e.eventType}`);
  console.log(`allEvents: element = ${e.element}`);
  if (e.eventType == 0) {
    console.log(`Window closed, open it again...`);
    myWindow.show(myHtml);
  }
}
WebUI.setTimeout(0);
WebUI.setMultiClient(true);
myWindow.bind("", allEvents);

// Show the window
myWindow.show(myHtml); // Or myWindow.show('./myFile.html');

// Wait until all windows get closed
await WebUI.wait();

console.log("Thank you.");
allEvents: window = [object Object]
allEvents: eventType = 1
allEvents: element =
allEvents: window = [object Object]
allEvents: eventType = 0
allEvents: element =
Window closed, open it again...
allEvents: window = [object Object]
allEvents: eventType = 1
allEvents: element =
allEvents: window = [object Object]
allEvents: eventType = 0
allEvents: element =
Window closed, open it again...
allEvents: window = [object Object]
allEvents: eventType = 1
allEvents: element =

@DevRubicate
Copy link
Contributor Author

Oh! I figured out the root cause of the issue.

If I have a window of Google Chrome open when I try the example, then WebUI uses Google Chrome to open it's popup. If I close this popup, it re-opens as expected.

image

However, if I don't have Google Chrome open when I try the example, then WebUI uses Firefox (my default browser) to open it's popup. If I close this popup, it fails and gives the error.

image

So it appears WebUI has some problem when using Firefox as it's browser. Also which browser you have running currently seems to affect what choice WebUI goes for.

@AlbertShown
Copy link
Contributor

You are right, I can reproduce the issue using Firefox.
I'm working on it.

Yes, webui use the running web browser for efficiency
But you can choose the browser you want myWindow.showBrowser(myHtml, WebUI.Browser.Firefox);

@DevRubicate
Copy link
Contributor Author

Makes sense, thanks for the explanation. 🙂

@AlbertShown
Copy link
Contributor

AlbertShown commented Dec 23, 2024

After testing and looking at logs, it seems to me that webui is too fast for Firefox...
webui tries to re-open a new Firefox window while Firefox still doing it's own thing to close first one...

To fix that, just add a few seconds delay before reopening and all will be fine.

  if (e.eventType == 0) {
    console.log(`Window closed, will reopen in 1 second...`);
    await new Promise(resolve => setTimeout(resolve, 1000)); // Sleep for 1 second
    myWindow.show(myHtml);
  }

@DevRubicate
Copy link
Contributor Author

DevRubicate commented Dec 23, 2024

(Unrelated, but I changed my username, but it's still me, Drakim)

Aha, good catch. I will do so.

But this could potentially confuse future users of WebUI who don't know to insert the setTimeout(). Isn't there a way to sorta see if firefox is busy and build the delay into the library? Or abort the request and try again if it fails a couple of times?

Either way, I know enough to work my way around the problem, thank you very much for investigating the issue.

@AlbertShown
Copy link
Contributor

I see your point, but it's better to make library general and not specific, I mean some users prefer the event to be fired fast and as soon as possible without any delay, other users like in your app you prefer a delay, so to make both scenarios working, the library fire the event as fast as it happen, then it's up to the user to deal with the event depend on his project needs.

I believe I saw a lot of official Microsoft C++ Win32 APIs example where Microsoft add delays in the example, while APIs stay fast as possible.

@DevRubicate
Copy link
Contributor Author

Hm, makes sense.

@DevRubicate
Copy link
Contributor Author

DevRubicate commented Dec 25, 2024

I've been testing a bunch, and here is what I've discovered:

  1. Using WebUI.setMultiClient(true); and the scriptClient() method does work neatly, so I can now open multiple windows, close them, and communicate with messages, this is good. It's a bit odd that I have to save an Event object as my main anchor of message communication instead of the actual constructed WebUI instance representing that window but it's no big deal. 😁

  2. Both firefox and chrome will produce the "unable to start the browser" occasionally, with firefox doing it a lot more frequently. I added in a setTimeout as you suggested, and that seems to help, however, it's not an exact science. Sometimes 1000ms is enough, sometimes it's not enough. To be safe I have to arbitrarily pad up all my user actions to always take two seconds to respond each click, this is a very sad user experience. 😟

  3. Sometimes the "unable to start the browser" error comes up even when there has been 10+ seconds between closing the last window and opening a new one. I assume it's based on external factors what's going on in the PC. Maybe sometimes the browser closes slower because it plans on applying an automatic update like modern browsers like to do. It's probably not your fault but WebUI suffers greatly as a tool because of it. 😟

  4. When the "unable to start the browser" error happens it always crashes my entire Deno app, it doesn't matter if I have a try catch around any of the WebUI functions I call. This means using WebUI is akin to playing Russian Roulette, any time I open a window it means my app may crash. This is a very sad user experience. 😟

I understand WebUI is still heavily under development, and that nobody can expect a perfectly stable product this early in development (WebUI website does say "In development...." for most things under Deno).

But these issues will be blockers for anybody wishing to try the library even for a trivial case. I've been happy to help out finding out issues and problems so far, but I will either have to pause my own project until WebUI is more mature or switch to some other library. I can't even show off my project as a prototype to somebody if clicks takes several seconds to apply and the app randomly crashes. 🥲

Still, thank you for the work so far. WebUI will be amazing once it's fully available on all platforms.

@AlbertShown
Copy link
Contributor

Yes, the Deno wrapper still under development. Thank you for reporting all those issues, I will fix them one by one soo.

  1. By default, multi-client option is disabled, means each window has one single client, therefore, you can simply use the window object to send JavaScript. But when you enable multi-client, means each window can have multiple clients, therefore, a window object is not enough, you need the client object to send JavaScript to a specific client, or use .run() which use window object to send JavaScript to all clients at once.
  2. Webui runs a simple command line to run the web browser, if browser fails to open, it's a browser issue, mostly this happens in Firefox as it does not support natively app mode. I suggest using Chromium based browsers, or switch to WebView mode (I will add it to Deno wrapper today).
  3. Same as 2
  4. I guess Deno is using throw new Error, and you can catch it using try{} catch (error) {}

In my opinion, browser mode is good for one single window project, while WebView is better for multi window projects.

@DevRubicate
Copy link
Contributor Author

DevRubicate commented Jan 4, 2025

  1. Oh, I see. So "window" refers to one instance of the browser. while "client" refers to one actual open viewport? The terminology is a bit confusing because to laypeople they would call the client a window, from typical usage like "popup window" which would be a new client but of the same window.

  2. Understood, and as I said, this is not WebUI's fault. It's just unfortunate that WebUI comes off as difficult and unstable to use due to browser problems, but such is life.

  3. No, try{} catch (error) {} does not help, Deno will still crash. I assume this is because some async scope issue means that the error does not happen in the scope of the caller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants