Skip to content

fix(analytics): filter cookies in Amplitude proxy to prevent request rejections#2476

Merged
ygrishajev merged 1 commit intomainfrom
feature/analytics
Jan 13, 2026
Merged

fix(analytics): filter cookies in Amplitude proxy to prevent request rejections#2476
ygrishajev merged 1 commit intomainfrom
feature/analytics

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Jan 12, 2026

Create custom API route handler for /api/collect that removes all cookies before proxying to Amplitude's API. This prevents Amplitude from rejecting requests due to excessive or irrelevant cookies while maintaining analytics functionality.

  • Add /api/collect API route that proxies to Amplitude with cookie filtering
  • Rewrite URL path from /api/collect to /2/httpapi to match Amplitude endpoint
  • Initialize proxy server once and reuse for better performance
  • Remove Next.js rewrite rule as API route takes precedence
image

Summary by CodeRabbit

  • New Features

    • Added a server-side API route that proxies external requests and exposes custom API runtime options.
  • Chores

    • Removed client-side environment validation and the conditional client-side proxy rewrite configuration.
    • Simplified runtime configuration by eliminating the prior env-driven rewrite behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@ygrishajev ygrishajev requested a review from a team as a code owner January 12, 2026 16:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

📝 Walkthrough

Walkthrough

Replaces Next.js rewrite-based Amplitude proxy with a new API route at /api/collect that memoizes a proxy server and forwards requests to Amplitude; removes environment validation and the conditional rewrite from next.config.js.

Changes

Cohort / File(s) Summary
Next.js config cleanup
apps/deploy-web/next.config.js
Removed browser environment validation and the conditional rewrites logic for NEXT_PUBLIC_AMPLITUDE_PROXY_URL; nextConfig no longer includes rewrites.
Amplitude proxy API
apps/deploy-web/src/pages/api/collect/index.ts
Added new API route that memoizes an HTTP proxy, strips cookies, rewrites incoming paths to /2/httpapi, forwards requests to Amplitude, and wires proxy events; exports config with api.externalResolver: true and api.bodyParser: false.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant NextAPI as Next.js /api/collect
    participant Proxy as Memoized Proxy
    participant Amplitude

    Client->>NextAPI: POST /api/collect (Amplitude payload)
    NextAPI->>NextAPI: strip cookies, set req.url = "/2/httpapi"
    NextAPI->>Proxy: forward request (changeOrigin, target=Amplitude)
    Proxy->>Amplitude: HTTP request to Amplitude /2/httpapi
    Amplitude-->>Proxy: response
    Proxy-->>NextAPI: proxyRes event
    NextAPI-->>Client: proxy response (resolve)
    alt Proxy error
      Proxy-->>NextAPI: error
      NextAPI-->>Client: 502 (if headers not sent)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • stalniy
  • baktun14

Poem

🐰 I hopped from config into a route so neat,
Requests now bounce through a tidy little suite,
Cookies trimmed, URLs set straight,
Proxy hops on to Amplitude's gate,
Hooray — a cleaner path for every event's feet! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an Amplitude proxy that filters cookies to prevent request rejections, which directly matches the primary functionality introduced in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c75e34 and 4716602.

📒 Files selected for processing (2)
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/pages/api/collect/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/pages/api/collect/index.ts
⏰ 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). (3)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Analyze (javascript-typescript)

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

@codecov
Copy link

codecov bot commented Jan 12, 2026

Codecov Report

❌ Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.61%. Comparing base (5a1ed1f) to head (4716602).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
apps/deploy-web/src/pages/api/collect/index.ts 0.00% 33 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (79.69%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2476      +/-   ##
==========================================
- Coverage   50.96%   50.61%   -0.36%     
==========================================
  Files        1062     1053       -9     
  Lines       29185    28869     -316     
  Branches     6450     6398      -52     
==========================================
- Hits        14875    14612     -263     
+ Misses      14079    13963     -116     
- Partials      231      294      +63     
Flag Coverage Δ *Carryforward flag
api 79.69% <ø> (ø) Carriedforward from 5a1ed1f
deploy-web 30.96% <0.00%> (-0.06%) ⬇️
log-collector ?
notifications 87.94% <ø> (ø) Carriedforward from 5a1ed1f
provider-console 81.48% <ø> (ø) Carriedforward from 5a1ed1f
provider-proxy 84.35% <ø> (ø) Carriedforward from 5a1ed1f

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
apps/deploy-web/src/pages/api/collect/index.ts 0.00% <0.00%> (ø)

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

baktun14
baktun14 previously approved these changes Jan 12, 2026
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: 2

🤖 Fix all issues with AI agents
In @apps/deploy-web/src/pages/api/collect/index.ts:
- Line 37: The current call to services.logger.error({ error, event:
"AMPLITUDE_PROXY_ERROR" }) won't let the logger extract stack/metadata because
it expects an Error instance (or specific key like err); update the call to pass
the Error directly or use the logger's err key: call
services.logger.error(error, { event: "AMPLITUDE_PROXY_ERROR" }) or
services.logger.error({ err: error, event: "AMPLITUDE_PROXY_ERROR" }) (or, if
neither is supported, log a descriptive message and then log the error
separately) so the stack and error details are captured; target the
services.logger.error invocation and the local error variable in the
AMPLITUDE_PROXY_ERROR handling.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a1ed1f and 2c75e34.

📒 Files selected for processing (2)
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/pages/api/collect/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{ts,tsx,js}: Never use type any or cast to type any. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.

Files:

  • apps/deploy-web/src/pages/api/collect/index.ts
  • apps/deploy-web/next.config.js
🧠 Learnings (1)
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.

Applied to files:

  • apps/deploy-web/next.config.js
🧬 Code graph analysis (1)
apps/deploy-web/src/pages/api/collect/index.ts (3)
apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
  • AppServices (63-63)
apps/deploy-web/src/lib/nextjs/defineApiHandler/defineApiHandler.ts (1)
  • defineApiHandler (13-57)
packages/logging/src/services/logger/logger.service.ts (1)
  • error (146-152)
⏰ 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: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (3)
apps/deploy-web/next.config.js (1)

161-163: LGTM!

Clean removal of the Amplitude proxy rewrites configuration. The functionality is now properly handled by the new API route at /api/collect, which allows for cookie filtering that wasn't possible with Next.js rewrites.

apps/deploy-web/src/pages/api/collect/index.ts (2)

4-17: LGTM on proxy initialization.

Good use of ReturnType for type inference avoiding explicit any. Memoizing the proxy server is appropriate for performance.


48-53: LGTM!

Correct configuration for a proxy route — externalResolver: true prevents Next.js warnings about unresolved requests, and bodyParser: false is necessary to stream the raw request body to the upstream.

stalniy
stalniy previously approved these changes Jan 12, 2026
…rejections

Create custom API route handler for /api/collect that removes all cookies
before proxying to Amplitude's API. This prevents Amplitude from rejecting
requests due to excessive or irrelevant cookies while maintaining analytics
functionality.

- Add /api/collect API route that proxies to Amplitude with cookie filtering
- Rewrite URL path from /api/collect to /2/httpapi to match Amplitude endpoint
- Initialize proxy server once and reuse for better performance
- Remove Next.js rewrite rule as API route takes precedence
@ygrishajev ygrishajev dismissed stale reviews from stalniy and baktun14 via 4716602 January 13, 2026 07:47
@ygrishajev ygrishajev merged commit 1245328 into main Jan 13, 2026
63 of 65 checks passed
@ygrishajev ygrishajev deleted the feature/analytics branch January 13, 2026 08:59
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

Comments