Throwing a thenable object in locks.requests callback will implicitly invoke its .then()

[ryanwang522]

Hi @inexorabletash , I have a question about throwing a "thenable" object (or returning a rejected promise with thenable object) in the web-locks api callback. "thenable" means that the object defines its .then() method.

Consider the following code,

var thenable = { 
    then: (...args) => { console.log("In thenable.then"); } 
} 

navigator.locks.request("lock-test", {mode: "exclusive"}, async () => {
    throw thenable; 
    // or return Promise.reject(thenable);
}).then((res) => console.log(res), (e) => console.log(e));

I am wondering is this an expected behavior?

Didn't find any spec mentioning this behavior. One might related is that if we return(resolve) a "thenable" object inside promise chain, it will unpack it by calling .then. It will call all .then's until it gets raw value.

I'm using Chrome 91.0.4472.106 (Official Build) (64-bit) on ubuntu 18.04.

[bathos]

I suspect you’ve found a Chromium bug. Neither Web IDL nor the internal ECMA-262 promise operations that Web IDL delegates to would access properties / invoke methods of a rejected Promise’s “reason” value — as you said, only fulfilling should lead to “unpacking”. I don’t think Web Locks is supposed to be doing anything unique in this regard.

[inexorabletash]

Agreed, looks like a Chrome bug. Can you file one via new.crbug.com ?

[ryanwang522]

@bathos Thanks for your clarification.
I've filed a ticket at https://bugs.chromium.org/p/chromium/issues/detail?id=1250253.

I am investigating chromium source code which might relates to the issue, will update on the above ticket if I found something suspicious, thanks guys :)

[inexorabletash]

Thanks. I made a note in the crbug about where to look in the code.

I'll leave this open for a bit; we should have a WPT for this.

[ryanwang522]

Hi @inexorabletash,

Sorry if I didn't express well in the above comment.
I think the issue comes from that Lock::ThenFunction is used as the onRejected handler of the lock's waiting promise (described in WICG spec, which is the promise returns from async callback).

When Lock::ThenFunction::Call() get invoked (to handle the promise rejection from the callback), it will return the thenable object and further spawn a resolve-thenable-job mircotask. That's why the .then() get called when throwing thenable object from the async callback.

I think the easiest approach to solve this is that we shouldn't return the thenable value in Lock::ThenFunction::Call(value) when the waiting promise has been rejected. How about returning a dummy ScriptValue() here if the promise is rejected?

I also left a comment (with the proposed WPT testcase for this) on crbug as well.

[inexorabletash]

Hey, let's keep the Chromium discussions on the Chromium bug. It's not a high priority issue so hasn't been given a lot of attention.

[saschanaz]

Can this be closed then?

[inexorabletash]

I think I left this open since we should add a WPT for this case.

[ryanwang522]

May I know what's the expected behavior of the to-do WPT from your perspective?
IIUC, the WPT should make sure that the then method of a thenable object should not be called when throwing/rejecting it from the async lock callback. For example:

  const test_thenable = { 
    then: (...args) => { 
      assert_unreached('Unexpected resolution of thenable object');
    }
  };
  const p = navigator.locks.request(res, async lock => { 
    throw test_thenable;
  });

However, this issue need to be fixed in Lock::ThenFunction first to meet the above behavior.

[saschanaz]

That test makes sense to me.

[inexorabletash]

Chromium CL up at https://chromium-review.googlesource.com/4064307 that includes a WPT

[inexorabletash]

The test case should be upstreamed soon, so closing this.