jasonkuhrt / graphql-request

Minimal GraphQL client
MIT License
5.75k stars 307 forks source link

Extension that can retry a hook #878

Closed jasonkuhrt closed 1 month ago

jasonkuhrt commented 1 month ago

Perceived Problem

Ideas / Proposed Solution(s)

Right now when an implementation fails (throws error) it will immediately stop the anyware stack.

This is a problem for the above use-case.

Instead, an extension should be able to receive the error and make another attempt at the hook.

This requires two new things:

  1. Hook execution must return error to caller. This means the result of a hook is no longer always the next hook. It could be an error. Forcing this all the time would be annoying. We could have a convention that only hooks that return errors (as specified in their return type) lead to the hook return type being union of that error(s) or the hook. Another approach could be that the envelop contains an error field. If we want to take this approach we need to allow Anyware constructor to accept information about hook returns, currently it formulates them based on the initial input or the next hook input. Inference is getting even more appealing with this additional spec requirement... Ask Pierre

  2. The ability to retry a hook. If there is an error then there must be a way to retry the hook from that point...

const result = await exchange()

if (result.type === 'error') {
  // refresh token
  await exchange() // have to get hook from here now? but what about success case?
}

But getting at the next hook becomes annoying and even unclear.

Perhaps instead, we need a retry within the hook itself. This would allow the return types of hooks to remain unchanged and not confuse getting the next hook into branches. The downside is it has to be an object (first parameter optional to allow auto-use of original input, making it not possible to ask for retry as second parameter).

const result = await exchange({ input: {...} })
const result = await exchange({ input: {....}, retry: (input) => ... })
const result = await exchange({ retry: (input) => ... })

We could have a context given that allows some insight into how many failures there have been but counting the errors length, an accumulating array of errors form each try.

const result = await exchange({ retry: (input, context:{ error: Error previousErrors: Error[], number:number ) => ... })

Retry would have the choice to rewrite input on every try in case that's needed, or just forward the input from the previous attempt.

We could have en envelop to allow signalling this is the last try.

const result = await exchange({ retry: (input, context:{ errors: [] ) => {
  return {
    lastAttempt: true,
    input,
  }
} })
jasonkuhrt commented 1 month ago

Thinking about if hooks can return result types. This would facilitate the flexibility for extensions to retry hooks arbitrarily. It would also permit other use-cases than retry, not specific to it.

// ...
let result = await exchange({....})
let i=0
while (result instanceof Error) {
  if (i > 3) return result // error
  i++
  result = await exchange({....})
}
const { unpack } = result
// ...

To opt out of error error handling users could use .throw() on the executed hook:

// ...
const { unpack } = await exchange({....}).orThrow()
// ...

When an extension would re-run a hook it would mean that all the extensions lower in the stack would execute again as well.

jasonkuhrt commented 1 month ago

After banging my head on this a few days I think I've realized what I want to do is impossible.

await run(
      async function foo({ a }) {
        const resultA1 = await a()
        const resultB1 = await resultA1.b() // imagine this is an error
        const resultB2 = await resultA1.b()
        return resultB2
      },
      async function bar({ a }) {
        const resultA1 = await a()
        const resultB1 = await resultA1.b()
        if (resultB1 instanceof Error) throw resultB1
        return resultB1
      },
    )

If extension foo does a retry on hook b there is no way for extension bar to then resume from await resultA1.b() which is what so far the ideas in this issue would assume would happen. Well... no it is not possible to "rewind" the function body like that.

The inability to retry a hook is a DX miss here... had the hooks been separate functions it would have been possible to add retry to any of them. The single function body approach seems to give up that potential.

If the extension is the last in the stack, it actually can retry because there is no rewinding of downstream functions required. What could a configuration API make that feature obvious and safe though?

Graffle
    .create()
    .extend(...)
    .extend(...)
    .extend({
      anyware: async (...) => { ... }, 
      anywareCatch: async ({ exchange }) => {
        let result = await exchange()
        if (result instance Error) result = ...
      }
    })

The typing for a catch extension will need to differ. Its hooks will return an either type that exposes the error for programatic control.

How would this look in Anyware? Just a new catch parameter I think...

anyware.run({
  initialInput,
  extensions,
  catch,
})