graphql-hive / graphql-hive/envelop
New API to always wrap execution phase
- Dominant language
- No language data
- Stars
- 827
- Forks
- 132
- PR merge metrics
- No merged PRs in 30d
Description
# 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:
```ts
const plugin: Plugin = {
onExecute({ setExecuteFn, executeFn }) {
setExecuteFn(async (...args) => {
// The plugin can do things just before the execution begins
const result = await executeFn(...args)
// And do things just after the execution has ended.
return result
})
}
}```
But this is fragile, since this API is very permissive and gives a lot of control over the execution phase.
The most problematic side effect is that if a plugin replace the execution function without wrapping the existing one, it will break any subsequent plugin also relying on wrapping the execution functon.
```ts
const wrappingPlugin: Plugin = {
onExecute({ setExecuteFn, executeFn }) {
setExecuteFn(async (...args) => {
console.time('execute')
const result = await executeFn(...args)
console.time('execute')
return result
})
}
}
const replacingPlugin: Plugin = {
onExecute({ setExecuteFn, executeFn }) {
setExecuteFn(myFancyCustomExecutor)
}
}
const yoga = createYoga({
plugins: [
wrappingPlugin,
replacingPlugin,
],
})
```
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 by `wrappingPlugin`.
To make it work, the plugin order should be reversed:
```ts
const yoga = createYoga({
plugins: [
replacingPlugin,
wrappingPlugin,
],
})
```
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:
```ts
const plugin: Plugin = {
onExecuteWrap(({ executed }) {
console.time('execute')
await executed // We wait for the execution phase to be done, instead of executing it directly
console.time('executed')
}
}
```
It would be possible to have access to potential errors:
```ts
const plugin: Plugin = {
onExecuteWrap(({ executed }) {
try {
await executed
} catch(err) {
console.error(err)
}
}
}
```
If needed, we can even give access to executionArgs and result:
```ts
const plugin: Plugin = {
onExecuteWrap(({ executionArgs, executed }) {
const result = await executed
console.time('executed', executionArgs, 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:
```ts
const plugin: Plugin = {
onExecuteMiddleware({ executionArgs, next }) {
const result = await next(executionArgs)
return result
}
}
```
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 with `setExecuteFn`. 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 the `next` 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.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.