Open EmrysMyrddin opened 7 months ago
To be able to set the execution result, I though of this API, but not sure if it's a good idea to allow this:
const plugin: Plugin = {
onExecuteWrap(({ executed }) {
const { result, setResult } = await executed
setResult(withMetadataExtesion(result))
}
}
The problem
Wrapping the execution of GraphQL operations is a very common need. For example, it is needed for tracing, monitoring, error handling, cache, etc...
Today, this is doable with the
setExecuteFn
API, which allow to entirely replace the execute function. By keeping a reference to the previous one, it is possible to wrap the execution phase:In this example, one would expect both the having a custom executor and the execution time log in the console. Actually, this is not working, because the
replacingPlugin
just overrides the wrapped execution function set bywrappingPlugin
. To make it work, the plugin order should be reversed:The plugins are order dependent, and the order can be know only by understanding the internal implementation details of the plugin, which is really not a good DX, especially for newcomers.
Solution proposal
A solution would be to have a dedicated hook for wrapping the execution phase. This would allow to create some kind of middleware.
To avoid having the same issues than
setExecuteFn
have, this API should enforce the fact that it can't cutoff the execution chain. All plugins should always be called for every execution. Otherwise, the point of having a new API would be purely a sort of "documentation", relying on the good will of plugin's maintainers to respect the rule.To enforce this, the responsibility of actually calling the execution function and all the plugin wrapping hooks should remain inside Envelop core.
To achieve that, we could rely on an execution Promise instead of an execution Function. The wrapper will have to await for a promise instead of directly calling the execution function. This way, if the wrapper doesn't await the given Promise, it's a bug in this plugin and it will not break other plugins in the chain.
Here an example of the previous plugin with this new API:
It would be possible to have access to potential errors:
If needed, we can even give access to executionArgs and result:
Perhaps we could even offer a way to set the result, but I'm not sure about this one.
Alternatives
An alternative would be to offer a classic middleware API like this:
But the problem with this approach is that nothing prevents the plugin to not call the
next
function, lead to the exact same problem we currently have withsetExecuteFn
. The main difference is that since this API is dedicated to wrapping, it would be considered a bug if a plugin breaks the expectation of calling thenext
function. But compared to the previous proposition, this bug would break the entires plugin chain.Additional context
This is in the context of multiple users having issues writing reliable tracing or monitoring plugins, such as the OTEL plugin.