chore: pass authToken to analytics query #7306
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe update removes explicit "Content-Type: application/json" headers from GET requests in analytics API functions and replaces the content-type header with an Authorization header using a bearer token in the shared fetch function. Debug code related to query parameters is also eliminated, with no changes to function signatures. Changes
Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/dashboard/src/@/api/analytics.ts (2)
29-49: Consider using URL constructor for more robust URL parsing.The manual URL parsing logic could be simplified and made more robust by using the URL constructor directly.
- const [pathname, searchParams] = input.toString().split("?"); - if (!pathname) { - throw new Error("Invalid input, no pathname provided"); - } - - // create a new URL object for the analytics server - const analyticsServiceUrl = new URL( - ANALYTICS_SERVICE_URL || "https://analytics.thirdweb.com", - ); - - analyticsServiceUrl.pathname = pathname; - for (const param of searchParams?.split("&") || []) { - const [key, value] = param.split("="); - if (!key || !value) { - throw new Error("Invalid input, no key or value provided"); - } - analyticsServiceUrl.searchParams.append( - decodeURIComponent(key), - decodeURIComponent(value), - ); - } + const inputUrl = new URL(input.toString(), 'http://dummy.com'); + const analyticsServiceUrl = new URL( + ANALYTICS_SERVICE_URL || "https://analytics.thirdweb.com", + ); + + analyticsServiceUrl.pathname = inputUrl.pathname; + analyticsServiceUrl.search = inputUrl.search;
220-221: Improve error logging consistency.Some functions include the response reason in error logs while others don't. Consider standardizing error logging across all analytics functions for better debugging.
- console.error("Failed to fetch RPC method usage"); + const reason = await res?.text(); + console.error( + `Failed to fetch RPC method usage, ${res?.status} - ${res.statusText} - ${reason}`, + );- console.error("Failed to fetch Engine Cloud method usage"); + const reason = await res?.text(); + console.error( + `Failed to fetch Engine Cloud method usage, ${res?.status} - ${res.statusText} - ${reason}`, + );Also applies to: 420-421
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/src/@/api/analytics.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/dashboard/src/@/api/analytics.ts (2)
24-27: LGTM: Proper authentication validation.The implementation correctly validates the auth token presence and throws a descriptive error when unauthorized. This prevents making requests without proper authentication.
54-54: LGTM: Correct Bearer token implementation.The Authorization header is properly formatted using the Bearer token scheme, which aligns with the PR objective to pass authToken to analytics queries.
| return fetch(analyticsServiceUrl, { | ||
| ...init, | ||
| headers: { | ||
| "content-type": "application/json", |
There was a problem hiding this comment.
Analytics-service only has GET queries. No content-type header is needed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7306 +/- ##
=======================================
Coverage 55.57% 55.57%
=======================================
Files 909 909
Lines 58673 58673
Branches 4158 4158
=======================================
Hits 32607 32607
Misses 25959 25959
Partials 107 107
🚀 New features to boost your workflow:
|
Merge activity
|
size-limit report 📦
|
[Dashboard] Fix: Update Authorization in Analytics API
Notes for the reviewer
This PR updates the authorization mechanism in the analytics API by:
How to test
Verify that analytics API requests are properly authorized and returning expected data.
Summary by CodeRabbit