Graphql-shield: Error thrown inside an async rule are not sent to the fallbackError function

[Sytten]

Describe the bug

If an async rule throws inside a chain, the error is not sent to the fallbackError function. The value is null.

Example

const editorIsEventOwner= chain(
  isPromoterStaff,
  rule({ cache: PermissionCache.STRICT })(
    async (_parent: {}, args: { id: string }, ctx: Context) => {
      const { id } = args;
      const event = await findEvent(toInt(id), ctx); // This can throw an Error
      if (!event) return false;
      return event.ownerId === ctx.user?.id;
    }
  )
);

This will correctly not allow the user to proceed if the findEvent fails, but it will not put the Error in the first parameter of the the function.

[open-collective-bot[bot]]

Hey @Sytten :wave:,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider contributing financially.

https://opencollective.com/graphql-shield

PS.: We offer priority support for all financial contributors. Don't forget to add priority label once you start contributing :smile:

[Sytten]

I created a work around in the mean time:

type AsyncIRuleFunction = (
  parent: any,
  args: any,
  ctx: any,
  info: any
) => Promise<IRuleResult>;

export const safe = (func: AsyncIRuleFunction): IRuleFunction => {
  return (parent: any, args: any, ctx: any, info: any) => {
    return func(parent, args, ctx, info).catch(
      (_) => new InternalServerError() // This inherits from ApolloError
    );
  };
};

[maticzav]

Could you create a reproduction CodeSandbox?

[Sytten]

@marktani here it is: https://codesandbox.io/s/example-graphl-shield-throw-x5g0h?file=/index.js

[Sytten]

Ok tracked down the issue. Basically here https://github.com/maticzav/graphql-shield/blob/master/src/rules.ts#L74 we return false for any error thrown so there is no way for the generator calling it to have this error. I will send a proposal that is not a breaking change (was hard to find, but I got one).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Zhouzi]

The workaround for us is to set debug to true in development. This way we can find out errors that are thrown instead of returned.

https://codesandbox.io/s/example-graphl-shield-throw-mfgle?file=/index.js

[Sytten]

I created a wrapper for the rule function. I would be willing to rewrite part of the lib to support more native feature if @maticzav let me. I basically did it with nexus-shield. I think the shift to throw would be better.

[maticzav]

That would be awesome @Sytten

[Sytten]

@maticzav I currently do not have a lot of time, but I would be happy to hop on a call so we can design the API and prepare for this v2 interface. Ping me on the prisma slack maybe?

[guiaramos]

Any update on this issue?