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

Support Promise as return value from webRequest.onAuthRequired #490

Open
Rob--W opened this issue Nov 14, 2023 · 4 comments
Open

Support Promise as return value from webRequest.onAuthRequired #490

Rob--W opened this issue Nov 14, 2023 · 4 comments
Labels
implemented: firefox Implemented in Firefox inconsistency Inconsistent behavior across browsers neutral: safari Not opposed or supportive from Safari supportive: chrome Supportive from Chrome

Comments

@Rob--W
Copy link
Member

Rob--W commented Nov 14, 2023

The webRequest.onAuthRequired event is the only webRequest event that can block a request asynchronously in Chrome (In Firefox, all webRequest events can be async; Safari does not support any blocking webRequest API).

Firefox and Chrome have an interoperable API when an extension wants to return the result synchronously from the event handler function of webRequest.onAuthRequired. But for asynchronous results, there is currently a large discrepancy, as I elaborated at mdn/content#30188 (review) (The permission aspect is out-of-scope for this issue, that's part of #264).

Chrome and Firefox support this syntax to handle an event synchronously:

// In Chrome: use chrome. instead of browser.
browser.webRequest.onAuthRequired.addListener(
  function (details) {
    return { /* BlockingResponse here*/ };
  },
  { urls: ["*://example.com/*"] },
  ["blocking"]
);

Firefox also supports asynchronous results through a Promise. In the above example, replace "function (details) {" with "async function (details) {`, and it would be effectively the same as the following. Note that the event registration looks identical to before:

browser.webRequest.onAuthRequired.addListener(
 function () {
    // Does NOT work in Chrome!
    return new Promise(resolve => {
      resolve({ /* BlockingResponse here*/ });
    });
  },
  { urls: ["*://example.com/*"] },
  ["blocking"]
);

Chrome does not support Promise as return value. Instead, it requires a different way to register the listener, and passing the result via a callback:

chrome.webRequest.onAuthRequired.addListener(
  function (details, asyncCallback) { // listener receives "asyncCallback" parameter.
    asyncCallback({ /* BlockingResponse here*/ });
    // Note: any return value is ignored.
  },
  { urls: ["*://example.com/*"] },
  ["asyncBlocking"] // "asyncBlocking" instead of "blocking"
);

I propose to update Chrome's implementation to support Promise, with the following remarks:

  • Preserve backcompat: Currently, when asyncBlocking is used, the return value is ignored. This should continue, in case extensions use async function and still call asyncCallback asynchronously after the Promise resolves.
  • "blocking" is currently documented to not support anything else than an object. Chrome could support Promise as a return value like Firefox, without concerns about backwards-compatibility (because Chrome extensions can currently not expect meaningful behavior when a Promise is returned).
  • For completeness: An alternative is to introduce yet a new extraInfoSpec enum value, e.g. "asyncBlockingPromise". I see no point in doing this.
@Rob--W Rob--W added the inconsistency Inconsistent behavior across browsers label Nov 14, 2023
@Rob--W
Copy link
Member Author

Rob--W commented Nov 14, 2023

FYI: Another example of an event handler that accepts a Promise in Firefox but requires a callback in Chrome is runtime.onMessage. Chrome expressed being in favor of accepting a Promise there at #338.

@xeenon xeenon added the neutral: safari Not opposed or supportive from Safari label Nov 15, 2023
@Rob--W Rob--W added implemented: firefox Implemented in Firefox supportive: chrome Supportive from Chrome labels Nov 23, 2023
@Rob--W
Copy link
Member Author

Rob--W commented Dec 11, 2023

I created an issue on Chromium's issue tracker for this, at https://bugs.chromium.org/p/chromium/issues/detail?id=1510405

@Rob--W
Copy link
Member Author

Rob--W commented Apr 5, 2024

For compatibility, Firefox is considering to also implement asyncBlocking + asyncCallback: https://bugzilla.mozilla.org/show_bug.cgi?id=1889897

@oliverdunk At the end of the initial issue, I sketched a potential way to support Promises in onAuthRequired with minimal impact on backward compatibility. Could you confirm which direction Chrome would prefer here? Ideally Firefox and Chrome should behave consistently here.

@oliverdunk
Copy link
Member

I spoke to the Chrome team about this and also had some chats with Rob. We are not keen on using asyncBlocking for this as it would be a breaking change to start giving meaning to the return value, especially in cases where you are using async functions that implicitly return a promise. I liked adding promise return support to blocking, but there is some concern about the implementation complexity. We are currently leaning towards the new asyncBlockingPromise value mentioned at the bottom of the initial issue but open to more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: firefox Implemented in Firefox inconsistency Inconsistent behavior across browsers neutral: safari Not opposed or supportive from Safari supportive: chrome Supportive from Chrome
Projects
None yet
Development

No branches or pull requests

3 participants