Is there a way to execute InnerModuleEvaluation 12.d.iv.3?

[codehag]

Hi, I have been trying to write a test that checks #152

However, I can never get this line to execute. EvaluationError is always undefined at this point, and is set later. I believe this is because the child modules must resolve before the async root resolves, but that is also true for module, the sub-parent.

The closest I got was with two children, both throwing errors after a promise execution. Does this line ever get executed or can it be deleted?

[guybedford]

Thanks for looking into these checks. It should be possible to hit this line with a graph like:

A -> B, C
B -> A
C -> A

where B and C are async, and B completes successfully but C completes with a failure. Follow by a secondary import / dynamic import later on of X which imports B directly.

When the new evaluation job for X imports B, it is important that we use the cycle root error from A for X, since the state of B strictly is going to be in evaluated state without any error since it executed under the acyclic mode of the async graph.

[jorendorff]

Here's my stab at writing a test for that scenario:

A.js

import B from "./B.js";
import C from "./C.js";
export default "A";

B.js

import A from "./A.js";
if (false) await 0;
export default "B";

C.js

import A from "./A.js";
if (false) await 0;
throw "FAIL";
export default "C";

X.js

import B from "./B.js";

test.js

async function test() {
  try {
    await import("./A.js");
    throw new Error("import should have failed");
  } catch (exc) {
    assertEq(exc, "FAIL");
  }

  try {
    await import("./X.js");
    throw new Error("import should have failed");
  } catch (exc) {
    assertEq(exc, "FAIL");
  }
}

test();

[jorendorff]

@guybedford This test case reaches the check... but then requiredModule.[[EvaluationError]] is undefined. Is the "return" reachable?

(Separately: in Node 15.7.0 this test case seems to work differently; B is marked as failed with "FAIL".)

[guybedford]

@jorendorff that test case looks exactly right, although is there definitely a task queue yielding happening before the error in the execution of C in that case? Perhaps the test should be changed to if (true) await 0 or even with a specific delay to ensure the scenario is checking the async error propogation after the post-order traversal has completed?

requiredModule.[[EvaluationError]] is undefined

This sounds quite odd - in the sync error propagation this should be set via the usual Evaluate () concrete method step 10 loop over the stack, and in the async error propagation case assuming requiredModule is the strongly connected component root, then by the parent propagation of the async callbacks.

[jorendorff]

@guybedford Oh, I see—stack will contain A, B, and C when the error occurs, because they're all in a strongly connected component. So B.[[Status]] remains evaluating with no error, until that whole SCC succeeds or fails.

I think this gives us enough to go on and this can be closed. Thanks so much for the clues.

[guybedford]

@jorendorff the stack only sticks around and is valid during the initial synchronous post-order traversal. So synchronous errors use that error setting approach. Asynchronous errors on the other hand propagate through separate callbacks on the async edges, which should be the error paths being handled here. The "async-evaluating" state that B is initially put in captures this difference. Happy to discuss further.

[guybedford]

I've added an amendment to the async rejection algorithm to reject cycles as a connected unit with the error placed on each entry together. This should result in this check no longer being necessary, hopefully alleviating the original confusion here. See https://github.com/tc39/proposal-top-level-await/pull/159/files#diff-181371b08d71216599b0acccbaabd03c306da6de142ea6275c2135810999805aR815 for the approach - we don't necessarily have to add this, but it did seem like something that came up as an implementation question.

[guybedford]

(Whether that PR can land is of course up to implementer consensus entirely at this point)

[codehag]

This has been fixed