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

Request for better documentation around window.loadURL #24113

Closed
3 tasks done
mix3d opened this issue Jun 12, 2020 · 15 comments
Closed
3 tasks done

Request for better documentation around window.loadURL #24113

mix3d opened this issue Jun 12, 2020 · 15 comments

Comments

@mix3d
Copy link

mix3d commented Jun 12, 2020

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for a feature request that matches the one I want to file, without success.

Problem Description

When using loadURL() and pages that take a while to finish loading, attempting to run loadURL() again on the same window results in a silent error. You can catch the error on the returned promise, but it does not tell you much either. (-2, "A generic failure occurred" from the chromium source)

Proposed Solution

The documentation should include text about how to handle that more.

Additionally, the link to the chromium error list definitions file no longer works:

Old: https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_error_list.h
Updated: https://source.chromium.org/chromium/chromium/src/+/master:net/base/net_error_list.h

Alternatives Considered

Spent numerous hours debugging why sometimes the page would load as expected, but sometimes load the initial URL used when creating the window only, but with no errors displayed. Was getting ready to refactor to instead of closing windows, listening to the close event and hiding the windows instead.

Additional Information

In my code, I'm creating windows ahead of time (so that the app can inflate itself and load state, which takes time), and re-creating them after the last child window is closed. They are initialized with one URL (an empty route that lets the app load without requesting much data), then right before being asked to show them I call loadURL() again which only changes the hash of the url and lets the SPA app reroute to load the needed items without reloading the page, making it appear to be very performant. (If I just created new windows every time, waiting for the 'ready-to-show' event caused a 2s delay between asking to show the window and actually showing it)

However, if the user attempts to open the child windows while the first loadURL() is in progress as part of their preloading initialization, a 2nd loadURL() call will silently fail.

Tested on v8.1.5, v8.2.3, and v9.0.1

@vladimiry
Copy link

See some info in #17526 and #19847.

@mix3d
Copy link
Author

mix3d commented Jun 15, 2020

So it's been around since it was introduced in v5, is "expected behavior", but many people say "expected behavior doesn't work for SPAs".

One suggestion I saw was to improve Electron by detecting in-progress loadURLs and queuing them internally. I think that would be a great solution.

@vladimiry
Copy link

vladimiry commented Jun 15, 2020

The SPA hash-based/same-page navigation case has been fixed by #18109 (before that window.on("load") ugly workaround was functional).

@mix3d
Copy link
Author

mix3d commented Jun 15, 2020

But in my case, I'm using loadURL twice from within the Main thread.

@vladimiry
Copy link

vladimiry commented Jun 15, 2020

If you only change the hash but the page remains the same then I guess #18109 fix is not complete for your case at least.

By the way I think you will need to provide a repro repository / project if you believe there is a bug in electron.

@mix3d
Copy link
Author

mix3d commented Jun 15, 2020

I've tried to reproduce without sharing our entire codebase, but the requirement of a "long loading page" makes a simple reproduction difficult, and thus the Electron Fiddle app was not very helpful.

My main goal with this issue was to address the lack of documentation around BrowserWindow.loadURL() and the potential pitfalls and/or workarounds for them.

@vladimiry
Copy link

but it does not tell you much either. (-2, "A generic failure occurred" from the chromium source)

I totally support you on this point. I also highlighted the issue of the error to be too generic which makes the developers spend/waste the time on digging what is the error cause.

@mix3d
Copy link
Author

mix3d commented Jun 15, 2020

But a -3 is aborted, which would still have told me something. -2 is literally "A generic failure occurred", which is what I was getting when attempting to loadURL() with a new route (and all that changed was the hash), while the first load was still in progress.

I was able to get around it by holding onto the promise from the first loadURL and waiting until it resolved before attempting to navigate again, but if Electron did this internally, that would be much better for the developer. (But I recognize that this could potentially add MORE pitfalls)

Additionally, if you loadURL but only the hash changes, a did-finish-load does not seem to fire.

@eternalharvest
Copy link

eternalharvest commented Mar 16, 2021

@mix3d, @sofianguy

I think this behavior is bug, so I submitted new issue for this.
When I pass option reloadIgnoringCache set to true, we can avoid this error.
But this options is not documented, and I think BrowserWindow#loadURL() should not throw an error even if the reloadIngnoreingCache option is not specified.

@mix3d
Copy link
Author

mix3d commented Mar 16, 2021

What's the issue number? Link these issues so the history can be preserved

@eternalharvest
Copy link

@mix3d #28208

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Oct 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2022
@lynn6020
Copy link

lynn6020 commented Mar 9, 2023

Is there a good solution to this problem? I also encountered this problem

@mix3d
Copy link
Author

mix3d commented Mar 13, 2023

According to this PR, it should be fixed in electron v20, v21, and v22, as of Oct 2022. I don't work for the company that ran into this issue anymore, and haven't touched Electron since, so I can't speak to whether or not it fixes my original issue.

IIRC, my previous solution was either a try-catch with a retry when it failed, or waiting for the first load to finish (by saving the promise in memory) before attempting to re-navigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants