- Notifications
You must be signed in to change notification settings - Fork 152
Fix Invalid time range issue #686
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
Conversation
WalkthroughReplaces bulk Docker Hub tag retrieval with a single latest-tag fetch that selects the amd64 digest (digest now optional) and updates the checkNewImageRelease flow. Also guards sidebar build-date formatting using isValid and shows "Not Available" when date is missing/invalid. Changes
Sequence Diagram(s)sequenceDiagram participant Sidebar participant API as /api/internal/version participant DockerHub Sidebar->>API: GET latest image status API->>DockerHub: GET /tags?ordering=last_updated&page_size=1 DockerHub-->>API: Latest tag metadata API->>DockerHub: GET /tags/{latest_tag} DockerHub-->>API: Tag images (multi-arch) API-->>Sidebar: TagCompressed { name, last_updated, digest? } Sidebar->>Sidebar: Format date if valid, else "Not Available" Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
web-server/pages/api/internal/version.ts (1)
148-150: Critical: serializing an invalid Date throws “Invalid time value” (likely the original crash). Return ISO strings or null instead.new Date(process.env.BUILD_DATE) is Invalid Date when the env var is missing or malformed. JSON serialization of an invalid Date calls toISOString and throws RangeError "Invalid time value". This can 500 the route and bubble as an error page on menu open.
Proposed changes:
- Return ISO strings (or null) for dates to avoid serialization pitfalls.
- Update the response type accordingly.
Suggested code (outside the changed lines):
// Change the response type type CheckNewVersionResponse = { latest_docker_image: string; github_repo: string; current_github_commit: string; is_update_available: boolean; latest_docker_image_build_date: string | null; current_docker_image_build_date: string | null; }; // In checkNewImageRelease(), replace the return’s date fields: const latestRemoteDate = new Date(latestTag.last_updated); const currentRaw = versionInfo.current_build_date; const currentParsed = currentRaw ? new Date(currentRaw) : null; return { // ... latest_docker_image_build_date: Number.isNaN(latestRemoteDate.getTime()) ? null : latestRemoteDate.toISOString(), current_docker_image_build_date: currentParsed && !Number.isNaN(currentParsed.getTime()) ? currentParsed.toISOString() : null };This aligns with your sidebar change: you can safely format or show “Not Available” based on presence and validity.
🧹 Nitpick comments (1)
web-server/pages/api/internal/version.ts (1)
132-132: Optional: add inline fallback to keep this route resilient even if fetchDockerHubLatestTag rejects.If you decide not to swallow errors in fetchDockerHubLatestTag, you can still guard here.
Apply this minimal diff:
- const latestTag = await fetchDockerHubLatestTag(); + const latestTag = await fetchDockerHubLatestTag().catch(() => ({ + name: 'unknown', + digest: 'unknown', + last_updated: new Date(0).toISOString() + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
web-server/pages/api/internal/version.ts(2 hunks)web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
web-server/pages/api/internal/version.ts (1)
web-server/http-server.js (1)
axios(5-5)
| @jayantbh We currently fetch all tags and pick the most recently updated one. Can you confirm that the latest tag will always point to the most recent version? |
| That's how it should be. Latest tag pointing to something else would just feel misleading. |
There was a problem hiding this 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
🔭 Outside diff range comments (1)
web-server/pages/api/internal/version.ts (1)
121-142: Prevent RangeError: “Invalid time value” by not serializing Date objects in the API response.Returning Date objects causes JSON serialization to call toISOString, which throws on invalid dates. This is likely the core of issue #685 when BUILD_DATE or last_updated is missing/invalid. Return ISO strings (or omit) instead.
Suggested changes (outside the changed hunk):
- Update response type to strings (optional) instead of Date.
- Return ISO strings conditionally.
Type change:
// Before type CheckNewVersionResponse = { latest_docker_image: string; github_repo: string; current_github_commit: string; is_update_available: boolean; latest_docker_image_build_date: Date; current_docker_image_build_date: Date; }; // After type CheckNewVersionResponse = { latest_docker_image: string; github_repo: string; current_github_commit: string; is_update_available: boolean; latest_docker_image_build_date?: string; current_docker_image_build_date?: string; };Return object change (inline, no new helpers needed):
return { latest_docker_image: latestDockerImageLink, github_repo: githubRepLink, current_github_commit: versionInfo.merge_commit_sha, is_update_available: isUpdateAvailable({ dockerLatestRemoteTag: latestTag, localVersionInfo: versionInfo }), latest_docker_image_build_date: isNaN(latestRemoteDate.getTime()) ? undefined : latestRemoteDate.toISOString(), current_docker_image_build_date: (() => { const d = new Date(versionInfo.current_build_date); return isNaN(d.getTime()) ? undefined : d.toISOString(); })() };Optional hardening (outside this hunk): Default envs to empty strings in getProjectVersionInfo to avoid type mismatches.
function getProjectVersionInfo(): ProjectVersionInfo { const merge_commit_sha = process.env.MERGE_COMMIT_SHA ?? ''; const build_date = process.env.BUILD_DATE ?? ''; return { merge_commit_sha, current_build_date: build_date }; }
♻️ Duplicate comments (1)
web-server/pages/api/internal/version.ts (1)
79-90: Add error handling and fallbacks for Docker Hub API and missing amd64 image.As previously noted, this can throw on network errors or unexpected payloads (images undefined/empty), violating “Handle failed API calls gracefully.”
Apply this diff to harden the function:
-async function fetchDockerHubLatestTag(): Promise<TagCompressed> { - const latestTagUrl = `https://hub.docker.com/v2/repositories/${dockerRepoName}/tags/${latestTagName}`; - const latestTag = (await axios.get<TagResult>(latestTagUrl)).data; - - const amdArchImage = latestTag.images.find((i) => i.architecture === 'amd64'); - - return { - name: latestTag.name, - digest: amdArchImage?.digest, - last_updated: latestTag.last_updated - }; -} +async function fetchDockerHubLatestTag(): Promise<TagCompressed> { + const latestTagUrl = `https://hub.docker.com/v2/repositories/${dockerRepoName}/tags/${latestTagName}`; + try { + const { data: latestTag } = await axios.get<TagResult>(latestTagUrl, { timeout: 10000 }); + const images = Array.isArray(latestTag.images) ? latestTag.images : []; + const amdArchImage = images.find((i) => i.architecture === 'amd64'); + return { + name: latestTag?.name ?? latestTagName, + digest: (amdArchImage ?? images[0])?.digest, + last_updated: latestTag?.last_updated ?? new Date(0).toISOString() + }; + } catch { + return { + name: latestTagName, + digest: undefined, + last_updated: new Date(0).toISOString() + }; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
web-server/pages/api/internal/version.ts(4 hunks)web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
web-server/pages/api/internal/version.ts (1)
web-server/http-server.js (1)
axios(5-5)
⏰ 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). (1)
- GitHub Check: All file linting
🔇 Additional comments (3)
web-server/pages/api/internal/version.ts (3)
8-8: Using the canonical 'latest' tag is acceptable; confirm this contract is stable across environments.Given product confirmation in the PR comments, hardcoding 'latest' is fine. If there's any chance of divergence in staging/EE images, consider making this configurable via env for safety.
63-67: Making digest optional is the right call; ensure all downstream code handles undefined.The new optional digest aligns with real-world data variability. Verify all consumers (links, UI, logs) treat an absent digest gracefully.
121-121: Simplifying to a single-tag fetch removes unnecessary calls and sorting.This streamlines the flow and reduces latency. Once the error handling is added in fetchDockerHubLatestTag, this path will be robust.
2cd5e79 to b14322c Compare There was a problem hiding this 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
🔭 Outside diff range comments (2)
web-server/pages/api/internal/version.ts (2)
21-28: Fix “Invalid time value” crash by returning ISO strings/null instead of Date objectsReturning Invalid Date objects causes JSON serialization to throw RangeError (root cause of the menu error). Return ISO strings or null; update the response type accordingly.
type CheckNewVersionResponse = { latest_docker_image: string; github_repo: string; current_github_commit: string; is_update_available: boolean; - latest_docker_image_build_date: Date; - current_docker_image_build_date: Date; + latest_docker_image_build_date: string | null; + current_docker_image_build_date: string | null; };
118-143: Action: Guard against null/invalid build dates in the UI and align API return typeShort: search shows the Sidebar constructs new Date(imageStatus?.current_docker_image_build_date) unconditionally (will turn null → 1970). Also the API code currently returns Date objects while the frontend type expects a DateString. Fix both so the UI only parses truthy/valid values and the API returns ISO string|null.
Files to change:
- web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx (around lines 70–73)
- Change to only call new Date(...) when the raw value is truthy and then use isValid, e.g.:
- const imageBuildDate = new Date(imageStatus?.current_docker_image_build_date); - const formattedDate = isValid(imageBuildDate) - ? format(imageBuildDate, 'dd MMM yyyy HH:mm:ss') - : 'Not Available'; + const rawBuildDate = imageStatus?.current_docker_image_build_date; + const imageBuildDate = rawBuildDate ? new Date(rawBuildDate) : null; + const formattedDate = + rawBuildDate && isValid(imageBuildDate) + ? format(imageBuildDate, 'dd MMM yyyy HH:mm:ss') + : 'Not Available';
- web-server/pages/api/internal/version.ts (types at ~lines 26–27 and return at ~140–142)
- If switching to ISO/null, return ISO strings or null (not Date objects). Example:
- latest_docker_image_build_date: latestRemoteDate, - current_docker_image_build_date: new Date(versionInfo.current_build_date) + latest_docker_image_build_date: latestRemoteDate.toISOString(), + current_docker_image_build_date: versionInfo.current_build_date || null,Notes / evidence from repo search:
- Frontend usage that needs the guard: web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx:70–73 (new Date(...) + isValid → used to render "Build: {formattedDate}").
- Frontend type already expects a string: web-server/src/types/resources.ts:1042 (current_docker_image_build_date: DateString).
- API file shows Date-typed fields and currently returns Date instances: web-server/pages/api/internal/version.ts (types and return values).
Please apply the Sidebar guard and ensure the API returns ISO string|null (and update version.ts types accordingly) so the UI shows "Not Available" instead of the epoch when the date is absent or invalid.
♻️ Duplicate comments (2)
web-server/pages/api/internal/version.ts (2)
125-128: Correct Docker Hub fallback URL — LGTMDefaulting to /r//tags and switching to the layers URL when a digest exists is correct. This fixes the earlier 404.
79-90: Guard Docker Hub API calls; add digest fallback and optional chaining on imagesNetwork/API errors and missing images can still crash this handler. Also,
.findonimageswill throw ifimagesis undefined. Wrap in try/catch, use optional chaining, and provide a digest fallback. This aligns with the acceptance criterion “Handle failed API calls gracefully.”-async function fetchDockerHubLatestTag(): Promise<TagCompressed> { - const latestTagUrl = `https://hub.docker.com/v2/repositories/${dockerRepoName}/tags/${latestTagName}`; - const latestTag = (await axios.get<TagResult>(latestTagUrl)).data; - - const amdArchImage = latestTag.images.find((i) => i.architecture === 'amd64'); - - return { - name: latestTag.name, - digest: amdArchImage?.digest, - last_updated: latestTag.last_updated - }; -} +async function fetchDockerHubLatestTag(): Promise<TagCompressed> { + const latestTagUrl = `https://hub.docker.com/v2/repositories/${dockerRepoName}/tags/${latestTagName}`; + try { + const { data: latestTag } = await axios.get<TagResult>(latestTagUrl, { timeout: 8000 }); + const amdArchImage = latestTag.images?.find((i) => i.architecture === 'amd64'); + const digest = + amdArchImage?.digest ?? + latestTag.images?.[0]?.digest ?? + latestTag.digest; + return { + name: latestTag.name ?? latestTagName, + digest, + last_updated: latestTag.last_updated ?? new Date(0).toISOString() + }; + } catch (_err) { + // Keep the API responsive even if Docker Hub is down or returns unexpected data + return { + name: latestTagName, + last_updated: new Date(0).toISOString() + }; + } +}
🧹 Nitpick comments (3)
web-server/pages/api/internal/version.ts (3)
110-116: Defensively handle invalid dates in update checkIf either date is invalid, the diff becomes NaN and the comparison is unreliable. Return false in that case.
- const localBuildDate = new Date(localVersionInfo.current_build_date); - const latestRemoteDate = new Date(dockerLatestRemoteTag.last_updated); - return ( - latestRemoteDate.getTime() - localBuildDate.getTime() > - estimatesBuildTimeInMs - ); + const localMs = new Date(localVersionInfo.current_build_date).getTime(); + const remoteMs = new Date(dockerLatestRemoteTag.last_updated).getTime(); + if (!Number.isFinite(localMs) || !Number.isFinite(remoteMs)) { + return false; + } + return remoteMs - localMs > estimatesBuildTimeInMs;
69-77: Avoid undefined env values; set safe defaultsPrevents type mismatches and simplifies downstream handling.
function getProjectVersionInfo(): ProjectVersionInfo { - const merge_commit_sha = process.env.MERGE_COMMIT_SHA; - const build_date = process.env.BUILD_DATE; + const merge_commit_sha = process.env.MERGE_COMMIT_SHA ?? 'unknown'; + const build_date = process.env.BUILD_DATE ?? '';
79-90: Optional: set a conservative axios timeoutPrevents the API from hanging on slow Docker Hub responses.
If you don’t adopt the larger try/catch refactor above, at least pass
{ timeout: 8000 }toaxios.gethere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
web-server/pages/api/internal/version.ts(4 hunks)web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web-server/src/layouts/ExtendedSidebarLayout/Sidebar/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
web-server/pages/api/internal/version.ts (1)
web-server/http-server.js (1)
axios(5-5)
⏰ 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). (1)
- GitHub Check: All file linting
🔇 Additional comments (2)
web-server/pages/api/internal/version.ts (2)
66-66: Making digest optional is appropriateMatches real-world API variability where a digest may be unavailable for a given architecture.
8-8: Constant for ‘latest’ tag — LGTMImproves readability and ensures consistency at call sites.
Linked Issue(s)
Fixes #685
Acceptance Criteria fulfilment
Comments
I have updated the method to get architecture info for that specific tag only. This api is more likely to send stable response even in future updates.
Also for any docker api changes as mentioned here, it won't affect our system.
Summary by CodeRabbit
Bug Fixes
Performance