graphql-hive / graphql-hive/envelop

Throwing an error from inside of `onExecute` should be handled as an execution error

Open
#918 6 comments 4 reactions 1 assignee Claimed by @saihaj View on GitHub
kind/enhancement
Dominant language
No language data
Stars
827
Forks
132
PR merge metrics
No merged PRs in 30d

Description

**Describe the bug**

If inside of an `onExecute` plugin, I do:
```typescript
export const myExecutePlugin = (): Plugin => {
return {
onExecute: async ({ executeFn, setExecuteFn }) => {
const newExecuteFn = async (args: TypedExecutionArgs) => {
throw new Error('Oh no! I couldnt execute');
};

setExecuteFn(newExecuteFn);
},
};
};
```

That error will be thrown all the way to the caller that called `getEnveloped().execute`.

**To Reproduce**

Code above

**Expected behavior**

Most of the time, the code that developers write will not expect `.execute` to potentially error, and this expectation is built into packages like `graphql-ws` and other GraphQL servers.

Instead, developers throughout the ecosystem have expected `result.errors` to be populated.

I propose that we wrap the final call to the executeFn wrapping all of the nested executeFn's in a try / catch statement here: https://github.com/dotansimha/envelop/blob/main/packages/core/src/orchestrator.ts#L463

And set `.errors` as expected.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.