Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add stale-while-revalidate to limits #906

Merged
merged 6 commits into from
Nov 8, 2023

Conversation

james-a-morris
Copy link
Contributor

@james-a-morris james-a-morris commented Nov 6, 2023

We should add stale-while-revalidate to our caching policy. The UX is improved by not requiring an unlucky user to request a fresh, uncached response.

Additionally, we have boilerplate on all endpoints to return a response. We should centralize our returned headers. This PR adds a small response function that is tested in limits.


Requirements Fulfilled:

  • Returned value on /limits is not more than 5 minutes
  • Recalculation is computed in the background.

Partially closes ACX-1692

Copy link

vercel bot commented Nov 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
frontend-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 7, 2023 7:21pm
goerli-frontend-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 7, 2023 7:21pm

api/_utils.ts Outdated Show resolved Hide resolved
api/limits.ts Outdated Show resolved Hide resolved
* @param cache The cache time in seconds - defaults to 300
* @returns The response object
*/
export function sendResponse(
Copy link
Contributor Author

@james-a-morris james-a-morris Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in this function is essentially copied/pasted throughout the codebase. I think it would make sense to centralize this as the redundancy makes it error-prone.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Is it worth setting some defaults on the input arguments? statusCode might be one where we could default to 200 right?

api/_utils.ts Outdated Show resolved Hide resolved
Copy link

linear bot commented Nov 7, 2023

ACX-1692 Look into occasional timeout in limits/suggested-fees

Occasionally we are receiving a timeout of longer than the 3 second requirement we expect from limits/suggested-fees. The initial assessment is this is a limitation of the underlying AWS lambda.

We can find some improvements by limiting the number of calls we make internally in these endpoints to other api/* routes.

api/_utils.ts Outdated Show resolved Hide resolved
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +1265 to +1267
statusCode: number,
cacheSeconds: number,
staleWhileRevalidateSeconds: number
Copy link
Contributor

@pxrl pxrl Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice if these were part of an object so they could be set by name, rather than inferred by order. Then you could probably nuke the comment as well.

options

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants