Async format error

[eltonio450]

Hi there,

This pull request is a first step to allow async formatError (and other extensions). It completes #1748 and closes #1747

Motivation

The motivation is for the lambda integration: a synchronous call to log an error is not satisfying, it needs to be awaited, otherwise the process is frozen before the resolution of the formatError promise. For exemple, if we want to send the error to an external service (sentry), it's never sent.

Tests - problems

There are still a few issues: express, hapi and koa integration are synchronous, and hence the tests cannot pass. Is there a way to enable this feature only for the other integrations ? Given the motivation, this feature is not really useful for those integrations.

Going further

I also saw in the code TODOs related to this synchronous -> asynchronous migration, is there a roadmap for thise ? Because of these synchronous integrations, there is a lot of code only to achieve retrocompatibility (isPromise, etc.)

What it does

What it does:

• the extension stack is unpiled, at every step the extension is checked: is it a promise or a synchronous call ? It's then passed to the next extension
• cache-controle and engine-reporting have been modified so that they can handle the case where an extension in the extension stack is a promise (not tested yet).
• a test has been written

IMO, in the future: the whole extension stack should work with Promises, and the entry variables should be converted with Promise.resolve(variables). The code would be more readable.

TODO:

• Update CHANGELOG.md with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

I do that as soon as I have guidance to fix this specific test issue :). What's the next step ?

Thank you !

Best,

[apollo-cla]

@eltonio450: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[abernix]

Closing for the same reason as #1748. There's nothing stopping you from kicking off async work which is un-awaited within formatError, but if you need to actually await async error work, please consider using the didEncounterErrors life-cycle hook which was recently added in #2719

For exemple, if we want to send the error to an external service (sentry), it's never sent.

This shouldn't be the case just because formatError isn't async. Anything on the event loop should still get finished! Please provide a reproduction if you're finding this to actually be the case. (Realistically, I don't think you'd want to hold the entire response contingent on sending a report to Sentry, that's certainly something that could just be fired off and finished after the request.)

[eltonio450]

Thanks for the tip, i'll look into didEncounterErros. The problem is that in a Lambda (AWS) environment, anything in the event loop is precisely frozen when the data is returned (and hence never executed).

I think lambda is a valid use case that has to be taken into account when designing such features :). This is the same problem for Apollo engine for example, where big requests are not logged into the engine, making the whole system unusable to debug too long calls (and that's why we don't pay for it for now...)