Skip to content

Conversation

@devksingh4
Copy link
Member

@devksingh4 devksingh4 commented Oct 27, 2025

For example, if/when us-east-2 goes down, we can still serve some semblance of data on core website routes.

Summary by CodeRabbit

  • Chores
    • Optimized HTTP caching policies across API endpoints to improve data retrieval performance and resilience.
    • Increased the revalidation window and introduced a configurable "stale-if-error" duration (1 day) to reduce failed-request impact and serve more reliable cached responses.

For example, if/when us-east-2 goes down, we can still serve some semblance of data on core website routes.
@github-actions
Copy link
Contributor

github-actions bot commented Oct 27, 2025

💰 Infracost report

Monthly estimate generated

This comment will be updated when code changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Added a new exported cache-duration constant and updated two API route cache-policy strings to use that constant and to double the stale-while-revalidate windows; no control-flow or error-handling changes.

Changes

Cohort / File(s) Summary
Configuration constant
src/common/config.ts
Added exported constant STALE_IF_ERROR_CACHED_TIME = 86400 (1 day).
Route cache policies
src/api/routes/events.ts, src/api/routes/organizations.ts
Imported STALE_IF_ERROR_CACHED_TIME and updated CLIENT_HTTP_CACHE_POLICY to use stale-if-error=${STALE_IF_ERROR_CACHED_TIME} and stale-while-revalidate=${... * 2} (doubled previous stale-while-revalidate windows).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check src/common/config.ts export and spacing/comment.
  • Verify correct imports and usage in src/api/routes/events.ts and src/api/routes/organizations.ts.
  • Confirm cache header string formatting and units (seconds) are consistent with expectations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Cache organizations and events longer in case of error" accurately describes the main objective of the changeset. The PR introduces a new STALE_IF_ERROR_CACHED_TIME constant set to 86400 seconds (1 day) and updates the cache policies for both organizations and events routes to use this longer error-cache duration, allowing the system to serve cached data when upstream services fail. The title is specific and clear, correctly identifying both the affected components (organizations and events) and the primary purpose (extended caching for error scenarios), which aligns directly with the PR's stated objective of serving cached data when us-east-2 or similar services go down.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 9fcc8bf and 7ef07ad.

📒 Files selected for processing (1)
  • src/common/config.ts (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 42bc160 and 9fcc8bf.

📒 Files selected for processing (3)
  • src/api/routes/events.ts (2 hunks)
  • src/api/routes/organizations.ts (2 hunks)
  • src/common/config.ts (1 hunks)
🧰 Additional context used
🪛 ESLint
src/common/config.ts

[error] 206-206: Insert ;

(prettier/prettier)

src/api/routes/events.ts

[error] 16-16: Unexpected use of file extension "js" for "../../common/config.js"

(import/extensions)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build Application
  • GitHub Check: Run Unit Tests
🔇 Additional comments (4)
src/api/routes/organizations.ts (2)

36-36: LGTM!

The import is correctly added to support the updated cache policy.


52-52: Well-structured cache policy for resilience.

The updated cache policy correctly implements the PR objective by using stale-if-error with a 1-day window, allowing stale content to be served during AWS outages. The dynamic stale-while-revalidate calculation (2x the base duration) is also a good practice.

src/api/routes/events.ts (2)

12-16: LGTM!

The import is correctly updated to include the new constant.


125-125: Excellent resilience improvement.

The cache policy change correctly implements extended caching during errors. With max-age=120, stale-while-revalidate=240, and stale-if-error=86400, events will remain accessible for up to 1 day during AWS outages while maintaining fresh data under normal conditions.

@devksingh4 devksingh4 merged commit 2f9bf91 into main Oct 27, 2025
6 of 11 checks passed
@devksingh4 devksingh4 deleted the dsingh14/error-cache-longer branch October 27, 2025 18:02
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.

2 participants