epicweb-dev / cachified

🤑 wrap virtually everything that can store by key to act as cache with ttl/max-age, stale-while-validate, parallel fetch protection and type-safety support
MIT License
916 stars 26 forks source link

Pass background tasks as Promise to new waitUntil option #103

Closed richardscarrott closed 5 months ago

richardscarrott commented 5 months ago

When running in a serverless environment such as Cloudflare Workers any background tasks such as cache revalidation and value migration aren't guaranteed to complete successfully because the serverless process is likely to be killed once it's handled the request.

This PR adds a new waitUntil option which allows call sites to pass in a callback accepting a promise representing background tasks.

In practice on CF Workers this would look like this:

export default {
  async fetch(request, env, ctx) {
    const result = await cachified({
      key: 'test',
      cache: lru,
      async getFreshValue() {
        const response = await fetch(
          `https://jsonplaceholder.typicode.com/users`,
        );
        return response.json();
      },
      ttl: 300_000,
      waitUntil: ctx.waitUntil.bind(ctx).
    });

    return new Response(result);
  },
};

https://developers.cloudflare.com/workers/runtime-apis/context/#waituntil

https://github.com/epicweb-dev/cachified/issues/71#issuecomment-2100172163

AdiRishi commented 5 months ago

Amazing PR, you tacked an issue that's been at the back of my mind for almost half a year now!

If you don't mind a question, why did you bind ctx to waitUntil in your example? Is this how you expect we would need to pass in that function? Looking at the code I would think it's enough to simply pass in ctx.waitUntil

richardscarrott commented 5 months ago

Hi @AdiRishi -- if you don't bind it blows up (or at least it did when I first tried workers ~a year ago) -- I presume it tries to reference itself on this.

Interestingly enough, if you're using CF Pages it's already bound -- I think here https://github.com/cloudflare/workers-sdk/blob/e57758f8ea39d296a6e11fac2c3629bf023ba850/packages/wrangler/templates/pages-template-worker.ts#L155 -- but if you're using Workers directly or CF Pages in advanced mode you need(ed) to bind.

Thanks for your work on the KV adapter -- just about to give it a go.

AdiRishi commented 5 months ago

Hi @AdiRishi -- if you don't bind it blows up (or at least it did when I first tried workers ~a year ago) -- I presume it tries to reference itself on this.

Very interesting, I'm curious to give this a shot myself and see what the behavior is like.

Thanks for your work on the KV adapter -- just about to give it a go.

Glad its turned out to be useful! If anything goes wrong or you find improvements to be made don't hesitate to create an issue / let me know 😄

github-actions[bot] commented 5 months ago

:tada: This PR is included in version 5.2.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: