Fixed CORS issues and update performance count fetching logic - #7943
Fixed CORS issues and update performance count fetching logic#7943FaizanCyber007 wants to merge 6 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDevelopment builds now proxy ChangesPerformance API integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PerformanceCounter
participant GatsbyMiddleware
participant CloudAPI
PerformanceCounter->>GatsbyMiddleware: Request performance count through /api/*
GatsbyMiddleware->>CloudAPI: Forward HTTPS request
CloudAPI-->>GatsbyMiddleware: Return response status, headers, and body
GatsbyMiddleware-->>PerformanceCounter: Relay response or return HTTP 502 JSON
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/sections/Meshery/Features-Col/index.js (1)
85-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStabilize
performanceCountEndpointfor the effect.
performanceCountEndpointis declared insideFeatures, but the effect has an empty dependency array at Line 110. Ifreact-hooks/exhaustive-depsis enabled, ESLint reports the endpoint reference as missing. Move the endpoint constant to module scope or include it in the dependency list.As per coding guidelines, run
npm run lintand ensure ESLint and Prettier checks pass.Also applies to: 102-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Meshery/Features-Col/index.js` around lines 85 - 90, Move performanceCountEndpoint outside the Features component to module scope, keeping its development and production values unchanged, so the effect’s empty dependency array no longer captures a component-local value. Run npm run lint and resolve any ESLint or Prettier issues.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gatsby-config.js`:
- Around line 57-64: Update the proxy middleware around app.use("/api/*") to
either narrow the route to the intended GET endpoint or fully proxy non-GET
requests by forwarding the request body and required headers, including
Content-Type, cookies, and authorization data, and writing the body before
proxyReq.end().
- Around line 59-75: Update the proxy request created in the https.request block
to configure proxyReq.setTimeout, destroy the stalled request when the timeout
fires, and return a 502 response only when res.headersSent is false. Preserve
the existing proxyReq error handling while applying the same headers-sent guard
before writing the failure response.
In `@src/sections/Counters/index.js`:
- Around line 19-24: Validate the performance API response before updating
state: in src/sections/Counters/index.js lines 19-24,
src/sections/Meshery/How-meshery-works/specs/data-card.js lines 50-55, and
src/sections/Projects/Nighthawk/index.js lines 27-32, require response.ok and a
finite result.totalRuns before calling setPerformanceCount; ensure invalid
status, malformed JSON, or missing values flow to the existing error handling.
In `@src/sections/Meshery/Features-Col/index.js`:
- Around line 102-108: Remove the performance-count assignment to 250000 in the
response.ok failure path, allowing the existing default value of 0 to remain
when the request fails. Preserve the error throw and the catch handling in the
performance-count fetch flow.
---
Nitpick comments:
In `@src/sections/Meshery/Features-Col/index.js`:
- Around line 85-90: Move performanceCountEndpoint outside the Features
component to module scope, keeping its development and production values
unchanged, so the effect’s empty dependency array no longer captures a
component-local value. Run npm run lint and resolve any ESLint or Prettier
issues.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9c86257-ae5a-41a6-a348-360c7b456146
📒 Files selected for processing (5)
gatsby-config.jssrc/sections/Counters/index.jssrc/sections/Meshery/Features-Col/index.jssrc/sections/Meshery/How-meshery-works/specs/data-card.jssrc/sections/Projects/Nighthawk/index.js
|
@KhushamBansal, please review it and help me to identify some more issues need to be fixed |
KhushamBansal
left a comment
KhushamBansal
left a comment
There was a problem hiding this comment.
@FaizanCyber007 Could you please address feedback by coderabbit?
Signed-off-by: Faizan Javed <faizanjaved246@gmail.com>
The proxy only forwards method and accept header, not body/Content-Type/ auth/cookies, so non-GET requests were silently broken. All current frontend usage of the relative /api/* path is GET-only, so scope the route accordingly instead of building out unused forwarding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Faizan Javed <faizanjaved246@gmail.com>
Add timeout + destroy on the dev proxy request and guard the error handler with res.headersSent so a mid-stream failure doesn't attempt a second response. Validate the performance API response (status and a finite totalRuns) before updating state in Counters, DataCard, and Nighthawk. Remove the hardcoded 250000 fallback in Features-Col so the default count of 0 is preserved on fetch failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Faizan Javed <faizanjaved246@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sections/Meshery/How-meshery-works/specs/data-card.js`:
- Around line 58-61: Update the totalRuns validation before setPerformanceCount
in the performance result handling to require a finite, non-negative integer,
rejecting negative and fractional values while preserving the existing
invalid-count error behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 66015fbe-a470-4716-8935-77d1cc2afe92
📒 Files selected for processing (5)
gatsby-config.jssrc/sections/Counters/index.jssrc/sections/Meshery/Features-Col/index.jssrc/sections/Meshery/How-meshery-works/specs/data-card.jssrc/sections/Projects/Nighthawk/index.js
💤 Files with no reviewable changes (1)
- src/sections/Meshery/Features-Col/index.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/sections/Projects/Nighthawk/index.js
- src/sections/Counters/index.js
- gatsby-config.js
02ac6b0 to
59173db
Compare
|
Preview deployment for PR #7943 removed. This PR preview was automatically pruned because we keep only the 3 most recently updated previews on GitHub Pages to stay within deployment size limits. If needed, push a new commit to this PR to generate a fresh preview. |
Number.isFinite allowed negative or non-integer values through to setPerformanceCount; require a finite, non-negative integer instead. Signed-off-by: Faizan Javed <faizanjaved246@gmail.com>
|
@KhushamBansal , I have implemented the changes suggested by Coderabbit. You can check them now and let me know |
| // so local `gatsby develop` sessions use the dev-only proxy from gatsby-config.js instead. | ||
| export const URL = | ||
| process.env.NODE_ENV === "development" | ||
| ? "/api/performance/results/total" |
There was a problem hiding this comment.
Heads-up: this endpoint is never locally served by the layer5.io website.
|
A duplicate of #7867? |
|
@leecalcote Confirmed — #7867, #7870, and this PR all fix the same root cause for #7860. Good catch. |
4 participants
Tile: Fix performance tests counter showing 0
Description
This PR fixes #7860
The "Performance Tests run" counter on the Meshery pages (
/cloud-native-management/meshery/,/cloud-native-management/meshery/operating-cloud-native-infra, and/projects/nighthawk) was permanently stuck at0instead of showing the live count (~160k+) fromhttps://cloud.layer5.io/api/performance/results/total.Root causes:
totalRuns(camelCase), but all four components readresult.total_runs(snake_case), which is alwaysundefined.Features-Col/index.js, the "cloud native integrations" counter and the "performance tests run" counter shared a single fallback condition (feature.count.value !== 0 ? feature.count.value : performanceCount). Whenever the integrations count is legitimately0(e.g. in a local lite dev build), it incorrectly displayed the performance count instead of its own value. Fixed by matching on the feature's identity (feature.count.description === "performance tests run") instead of a zero-sentinel.Counters/index.js,data-card.js, andNighthawk/index.jshad no.catch()on their fetch chains, so any fetch failure (including CORS, see below) left the counter at0with zero console output.Files changed:
src/sections/Counters/index.jssrc/sections/Meshery/Features-Col/index.jssrc/sections/Meshery/How-meshery-works/specs/data-card.jssrc/sections/Projects/Nighthawk/index.jsgatsby-config.jsNotes for Reviewers
While verifying this locally, the field-name fix alone wasn't enough to actually see working —
cloud.layer5.io's CORS policy only allowshttps://layer5.io/https://www.layer5.ioas origins, so a browser fetch fromlocalhost(any contributor, any machine) is silently blocked. This is unrelated to the bug above and doesn't affect production (already correctly allowed), but it meant no one could visually confirm this fix locally.To fix that,
gatsby-config.jsadds a dev-only proxy (developMiddleware, gated behindNODE_ENV === "development") that routes/api/*requests through the dev server's own origin, avoiding the cross-origin request entirely. It's implemented with Node's built-inhttpsmodule rather than Gatsby's own built-inproxyconfig, because that built-in feature has an existing bug in this Gatsby/Node version combination — itsreq.pipe(got.stream(...))wiring breaks TLS certificate verification for GET requests (reproduced deterministically while debugging this).This proxy has zero effect on
gatsby build/production:NODE_ENV=productionbuild and confirmed the built JS bundles contain the directcloud.layer5.ioURL with no trace of the dev-only proxy anywhere inpublic/.gatsby serve) and confirmed all affected pages return 200 with no runtime errors.Verified the fix visually against the dev server with a real Chrome instance — the performance counter genuinely animates from
0up to the live API value, and the three counters on the Meshery page ("cloud native integrations", "standardized components", "performance tests run") now render independently instead of collapsing onto the same number. Recording attached.Note: the "cloud native integrations" counter itself needed no change — it already renders the correct live count (~390) in production (verified directly against the production site's data). It shows
0in local lite dev builds only, becausedevelop:liteintentionally excludes theintegrationscontent collection to keep local builds fast — pre-existing, unrelated to this fix.Demo Video:
Recording.2026-08-10.021639.mp4
Signed commits
Summary by CodeRabbit
New Features
Bug Fixes
Style