Change rethrowAsync()'s behavior in test mode

[lannka]

To follow the conversation here

From a testing point of view, removing rethrowAsync will greatly reduce the amount of flakiness we see. However, is there a valid use case for doing it in the runtime? Why was it added in the first place? Does it prevent the runtime from blocking when it encounters an error, perhaps?

Can we change rethrowAsync to console.error() + reportErrorToServer()

/cc @erwinmombay @jridgewell @dvoytenko @choumx @rsimha

[jridgewell]

Copying @dvoytenko's #20233 (comment):

@rsimha Not sure if this is the best forum for rethrowAsync discussion, but to kick it off. The original intent with rethrowAsync was this: "Don't block the code, but still treat it as an error downstream, including reporting to console, analytics services, diagnostics logging". I realize that it caused us some paint with tests, but I think we could try to address that pain in a couple of different ways and maybe that'd be an improvement over the current state. E.g. something like:

function rethrowAsync(error) {
  if (TESTING) {
    // E.g. these errors could be then used for test assertions.
    TESTING.collectErrors(error);
  } else {
    // Normal PROD behavior.
    setInterval(() => {throw error;});
  }
}

With something like this, the if (TESTING) part would be stripped from PROD binary.

Overall, IMHO, I prefer rethrowAsync to user().log() and dev().log(). I'd prefer we relied on logging packages as little as possible. rethrowAsync is essentially a proxy to throw new Error() and reads much cleaner. But that's a bigger conversation.

[jridgewell]

I'm also supportive of rethrowAsync, for exactly the reasons @dvoytenko's has above.

[lannka]

Right, I do see the value of throwing an error (stack trace etc).

Can you explain what logging packages is?

[jridgewell]

src/log.js

[lannka]

SG, let's go with @dvoytenko's proposal.

@torch2424 since you will be working on the amp-experiment soon, it will be very important to unmute the tests. Assigning to you.

[ampprojectbot]

This issue doesn't have a category which makes it harder for us to keep track of it. @torch2424 Please add an appropriate category.

[rsimha]

This is another worthwhile effort that fits under the umbrella of #14360.

[rileyajones]

I can't seem to reproduce this issue anymore. If anyone thinks this is still happening, please reopen this ticket.
@dvoytenko @lannka