Edge: properly handle initialization failures and abortion #1578
Conversation
HeikoKlare
force-pushed
the
edge-initialization-abortion
branch
from
c35cfc6 to
86c452d
Compare
HeikoKlare
force-pushed
the
edge-initialization-abortion
branch
3 times, most recently
from
7d26fe6 to
8f4589a
Compare
|
@amartya4256 I cannot add you as a reviewer via GitHub tooling, but maybe you can still have a look at this one? |
There was a problem hiding this comment.
The changes look good to me. I have just added 2 suggestions. Also I came across these: MicrosoftEdge/WebView2Feedback#2495, MicrosoftEdge/WebView2Feedback#3008
It might be intersting for the error code 139f.
| System.err.println(String.format("Edge initialization failed, retrying (attempt %d / %d)", previousAttempts + 1, MAXIMUM_CREATION_RETRIES)); | ||
| createInstance(previousAttempts + 1); | ||
| } else { | ||
| System.err.println(String.format("Aborting Edge initialiation after %d retries", MAXIMUM_CREATION_RETRIES)); |
There was a problem hiding this comment.
Wondering if we should also throw an error here since the initialization was not successful.
There was a problem hiding this comment.
Yes, makes sense. I adapted this to throw an error instead of just printing an error message with ca41f1e.
| @@ -457,6 +457,7 @@ public class OS extends C { | |||
| public static final int EN_CHANGE = 0x300; | |||
| public static final int EP_EDITTEXT = 1; | |||
| public static final int ERROR_FILE_NOT_FOUND = 0x2; | |||
| public static final int ERROR_INVALID_STATE = 0x139F; | |||
There was a problem hiding this comment.
isn't 0x8007139F also a COMException and hence should be put in COM.java?
Refer these: MicrosoftEdge/WebView2Feedback#2495, MicrosoftEdge/WebView2Feedback#3008
There was a problem hiding this comment.
ERROR_INVALID_STATE == 0x139f is an ordinary Windows error code (see https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--4000-5999-), just like the other ordinary Windows error codes we have in OS (like ERROR_NO_MORE_ITEMS). So I would say it is correctly placed in OS.
There was a problem hiding this comment.
Oh alright then its all good
HeikoKlare
force-pushed
the
edge-initialization-abortion
branch
from
8f4589a to
38178e9
Compare
There is currently only few handling for failures during initialization of an Edge browser / WebView instance. The Microsoft documentation provides further information on this, in particular: - ERROR_INVALID_STATE indicates that multiple Edge instances with the same data folder but different environment options exist - E_ABORT indicates an active abortion of the initialization process - On any other non-OK return value than the above ones, the app should retry initialization of the instance; to avoid endless waiting, the number of retries is currently limited to 5 This change adds appropriate handling for these scenarios, consisting of uniform rollback logic when the initialization fails or is aborted and retry logic to make repeated creation attempts. Contributes to eclipse-platform#1664
HeikoKlare
force-pushed
the
edge-initialization-abortion
branch
from
38178e9 to
ca41f1e
Compare
|
Thank you for your reviews! |
There is currently only few handling for failures during initialization of an Edge browser / WebView instance. The Microsoft documentation provides further information on this, in particular:
This change adds appropriate handling for these scenarios, consisting of uniform rollback logic when the initialization fails or is aborted and retry logic to make repeated creation attempts.
Contributes to #1664