fix: don't cache rejected promises

[BeLi4L]

Current issue:
Sometimes our app would be stuck due to the following error in our logs: "Error: The request to Forest Admin server has timed out while trying to reach https://api.forestadmin.com/liana/v4/permissions/environment".
This would make our app unusable (since it can't reach Forest) and we had to restart the pod to make it work again.

Root cause:
The code responsible for calling the "https://api.forestadmin.com/liana/v4/permissions/environment" URL, in ActionPermissionService, uses a TTLCache, which keeps returning the same timed out promise.

Suggested solution:
Don't cache rejected promises, so that the following calls will retry until one succeeds.

[BeLi4L]

Another solution would have been to catch the "Time out" error only in ActionPermissionService, and invalidate the cache there, but I thought it would make sense to put this logic in TTLCache.

Let me know what you think, since I am not that familiar with this project 😉

[Scra3]

Hello @BeLi4L,
Thank you so much for your contribution! Your code seems to address a critical bug, and I truly appreciate your effort. I will present your contribution to the tech team in the coming weeks, as many of us are currently on holiday.
Once again, thank you for your help—it’s highly valued and greatly appreciated! 🙏

[jeffladiray]

Thanks a lot for this bugfix - We've been on this issue for weeks now, not being able to reproduce it on our internal agent.

Manually tested by forcing utils/server.ts to reject via a simulated timeout

      if (
        !path.includes('ip') &&
        !path.includes('hashcheck') &&
        !path.includes('oidc') &&
        !path.includes('liana/v2/renderings') &&
        !path.includes('permissions/users')
      ) {
        const err = new Error();
        err.timeout = true;
        throw err;
      }

Cache is indeed cleared for the specific key on rejected promise 🙌

(I'm still curious to understand why these timeout appears. Default timeout is ~10s, and all of our monitoring systems seems to indicate pretty good response time (P99>200ms), especially on users/environments set of permissions 🤔)

The CI is failling because of unaccessible credentials from external contribution, so I'll bypass it. Thanks a lot 🙏

[forest-bot]

🎉 This PR is included in version 1.12.2 🎉

The release is available on [email protected]

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.12.13 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.56.2 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.36.2 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[BeLi4L]

(I'm still curious to understand why these timeout appears. Default timeout is ~10s, and all of our monitoring systems seems to indicate pretty good response time (P99>200ms), especially on users/environments set of permissions 🤔)

The TTL is an option given to TTLCache, and comes from the options-validator file:

// One year cache duration when using events
const DEFAULT_CACHE_DURATION_WITH_EVENTS = 31560000;

// [...]

// When using the event source to refresh cache we set a one year cache duration
copyOptions.permissionsCacheDurationInSeconds = copyOptions.instantCacheRefresh
  ? DEFAULT_CACHE_DURATION_WITH_EVENTS
  : copyOptions.permissionsCacheDurationInSeconds ?? DEFAULT_MINIMUM_CACHE_DURATION * 15;

I have no idea why this is done, especially since the instantCacheRefresh naming would hint the opposite 😅

[forest-bot]

🎉 This PR is included in version 1.3.29 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.58.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.1.44 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.8.9 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.5.9 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.11.4 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.16.4 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.46.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.4.12 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.1.18 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[forest-bot]

🎉 This PR is included in version 1.4.2 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[BeLi4L]

FYI we still had the issue in prod, since we use forest-express-sequelize, which uses forest-express, which still uses "forestadmin-client" "1.35.0.

Maybe it'd be worth it to auto-update that lib too, when forestadmin-client is updated 😉

EDIT: I did the update there, in this PR: ForestAdmin/forest-express#1045