Skip to content

feat: serve amiusing.requestly.io response inline from the proxy#99

Merged
wrongsahil merged 2 commits into
masterfrom
feat/amiusing-builtin-interception
May 19, 2026
Merged

feat: serve amiusing.requestly.io response inline from the proxy#99
wrongsahil merged 2 commits into
masterfrom
feat/amiusing-builtin-interception

Conversation

@dinex-dev
Copy link
Copy Markdown
Member

@dinex-dev dinex-dev commented May 14, 2026

Summary

  • Replaces AmisuingMiddleware's header-injection with a direct response substitution: requests to amiusing.requestly.io are answered locally with a self-contained Success HTML page, without ever forwarding upstream.
  • Mirrors ssl_cert_middleware's pattern — runs as a pre-rules middleware via init_amiusing_handler, short-circuiting before the rule engine is consulted. No dependency on the rule system, the rule schema, or the requestly-core BLACK_LIST_DOMAINS check.
  • HTML body (carrying the existing two-column "Yes" design + a {"amiusing": true} JSON marker) is extracted to amiusing_yes_page.js for readability; no build changes — tsc handles the new module naturally.
  • Version bump to 1.3.15.

Why

Today the "Yes" indicator on amiusing.requestly.io depends on the proxy injecting a header that the origin's Next.js app reads. Once the origin migrates off Heroku to a static page (RQ-2153), the header path stops working. Serving the response inline from the proxy is durable across that migration and any future origin changes.

Test plan

  • Built locally (npm run build).
  • Smoke-tested via a linked install in requestly-desktop-app: through the desktop proxy, curl -x http://localhost:8281 http://amiusing.requestly.io returns the Success HTML; the request never reaches origin.
  • Confirmed the rule engine doesn't fire on these requests (no "Rules Applied" indicator in the desktop network log).
  • Verify CI passes.
  • Publish to npm (1.3.15) before the companion desktop-app dep bump can merge.

Follow-up

  • Desktop-app dep bump: companion PR in requestly/requestly-desktop-app.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a confirmation page that displays when Requestly is detected. The page includes success messaging, troubleshooting documentation links, and resources to explore Requestly's features.

Review Change Stack

Replaces the existing AmisuingMiddleware header-injection behaviour with a
direct response substitution: when a request hits amiusing.requestly.io, the
proxy writes a self-contained "Success" HTML page back to the client and
never forwards upstream. The page mirrors the existing amiusing.requestly.io
two-column "Yes" design (Success column + What's next doc links) and carries
a JSON marker (`{"amiusing": true}`) for programmatic detection.

This runs as a pre-rules middleware (registered via init_amiusing_handler),
so it short-circuits before the rule engine is consulted — the same pattern
ssl_cert_middleware uses for /ssl certificate downloads. It does not depend
on the rule system or on any desktop-app side changes beyond a dep bump.

Effect: with this proxy version installed, any client routed through the
desktop proxy that hits amiusing.requestly.io sees the Success page,
independent of what the origin currently serves. Once the origin migrates
to a static "No" page, only stale proxies still depend on the origin path.

The HTML body is extracted to amiusing_yes_page.js to keep the middleware
file focused on routing/handler logic. Bumps version to 1.3.15.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Walkthrough

The PR adds an "Am I Using Requestly" feature to the proxy middleware. A new file exports AMIUSING_YES_HTML, a static HTML template containing responsive layout styling, status messaging, troubleshooting documentation links, and an embedded JSON marker. The middleware now intercepts requests to amiusing.requestly.io, writes a 200 response with the HTML body and no-store caching directive, includes an amiusingrequestly: "true" response header, and short-circuits without upstreaming the request.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: serving amiusing.requestly.io responses directly from the proxy instead of relying on header injection or upstream forwarding.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/amiusing-builtin-interception

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 14, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@requestly/requestly-proxy@99

commit: 2adac48

Copy link
Copy Markdown

@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

🤖 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/components/proxy-middleware/middlewares/amiusing_middleware.js`:
- Around line 24-25: The host check is too strict and will miss matches when the
incoming host includes a port or different casing; update the condition that
uses ctx.proxyToServerRequestOptions.host and AMIUSING_HOST to compare
normalized hostnames by extracting the hostname (strip any port portion after
':' and remove surrounding IPv6 brackets if present), lowercasing and trimming
both sides, then compare equality (e.g., normalize incomingHost =
stripPortAndBrackets(ctx.proxyToServerRequestOptions.host).toLowerCase() and
compare to AMIUSING_HOST.toLowerCase()); return early only if they do not match.

In `@src/components/proxy-middleware/middlewares/amiusing_yes_page.js`:
- Line 86: Update the user-facing heading text in the AMI using "yes" page:
replace the current h2 content "What's next you ask ?" with properly punctuated
copy "What's next, you ask?" in the component that renders the <h2>
(amiusing_yes_page.js) so the displayed heading uses the comma and moves the
question mark directly after the phrase.
🪄 Autofix (Beta)

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 36c346f1-4473-4a69-b2dc-815fee3420ac

📥 Commits

Reviewing files that changed from the base of the PR and between fcf19c0 and 7ad2da6.

⛔ Files ignored due to path filters (3)
  • dist/components/proxy-middleware/middlewares/amiusing_middleware.js is excluded by !**/dist/**
  • dist/components/proxy-middleware/middlewares/amiusing_yes_page.d.ts is excluded by !**/dist/**
  • dist/components/proxy-middleware/middlewares/amiusing_yes_page.js is excluded by !**/dist/**
📒 Files selected for processing (3)
  • package.json
  • src/components/proxy-middleware/middlewares/amiusing_middleware.js
  • src/components/proxy-middleware/middlewares/amiusing_yes_page.js

Comment thread src/components/proxy-middleware/middlewares/amiusing_middleware.js
Comment thread src/components/proxy-middleware/middlewares/amiusing_yes_page.js
Comment thread src/components/proxy-middleware/middlewares/amiusing_middleware.js
Comment thread package.json Outdated
Addresses review feedback on this PR:

- Echo `amiusingrequestly: true` as a response header on the synthesized
  Success response, so any downstream consumer that historically inspected
  the legacy request-side marker continues to see it after we stopped
  forwarding upstream.

- Revert package.json version 1.3.14 → 1.3.15 bump; the release pipeline
  handles version bumps automatically.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@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

🧹 Nitpick comments (1)
src/components/proxy-middleware/middlewares/amiusing_middleware.js (1)

28-36: ⚡ Quick win

Consider adding error handling around response operations.

While the static HTML content is unlikely to cause issues, adding a try-catch around the response write operations would improve resilience in case of unexpected errors (network issues, client disconnection, etc.).

🛡️ Optional defensive wrapper
+    try {
       ctx.proxyToClientResponse.writeHead(200, {
         "Content-Type": "text/html",
         "Cache-Control": "no-store",
         // Echo the legacy request-side marker as a response header too so any
         // downstream consumer that historically inspected `amiusingrequestly`
         // continues to see it after we stopped forwarding upstream.
         "amiusingrequestly": "true",
       });
       ctx.proxyToClientResponse.end(AMIUSING_YES_HTML);
+    } catch (err) {
+      console.error("[AmisuingMiddleware] Failed to send response:", err);
+      // Response already started or client disconnected; nothing more to do
+    }
🤖 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/components/proxy-middleware/middlewares/amiusing_middleware.js` around
lines 28 - 36, Wrap the response write operations that use
ctx.proxyToClientResponse.writeHead and
ctx.proxyToClientResponse.end(AMIUSING_YES_HTML) in a try-catch to handle
unexpected errors (e.g., client disconnects or network issues); on error, check
ctx.proxyToClientResponse.headersSent or presence and attempt a safe cleanup
(log the error via your logger and call ctx.proxyToClientResponse.end() if
possible) to ensure the connection is closed gracefully and no uncaught
exceptions bubble up.
🤖 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/components/proxy-middleware/middlewares/amiusing_middleware.js`:
- Line 10: The file uses ES6 import syntax for AMIUSING_YES_HTML which conflicts
with the repo's CommonJS setup; fix by replacing the ES module import with a
CommonJS require (use const { AMIUSING_YES_HTML } =
require("./amiusing_yes_page")) in
src/components/proxy-middleware/middlewares/amiusing_middleware.js so the source
matches the project's "module": "commonjs" and "type": "commonjs" configuration,
or alternatively update tsconfig.json ("module": "esnext") and package.json
("type": "module") if you intend to switch the whole project to ESM.

---

Nitpick comments:
In `@src/components/proxy-middleware/middlewares/amiusing_middleware.js`:
- Around line 28-36: Wrap the response write operations that use
ctx.proxyToClientResponse.writeHead and
ctx.proxyToClientResponse.end(AMIUSING_YES_HTML) in a try-catch to handle
unexpected errors (e.g., client disconnects or network issues); on error, check
ctx.proxyToClientResponse.headersSent or presence and attempt a safe cleanup
(log the error via your logger and call ctx.proxyToClientResponse.end() if
possible) to ensure the connection is closed gracefully and no uncaught
exceptions bubble up.
🪄 Autofix (Beta)

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0a647aab-a7f3-443d-9498-7ae652f8411b

📥 Commits

Reviewing files that changed from the base of the PR and between 7ad2da6 and 2adac48.

⛔ Files ignored due to path filters (1)
  • dist/components/proxy-middleware/middlewares/amiusing_middleware.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • src/components/proxy-middleware/middlewares/amiusing_middleware.js

Comment thread src/components/proxy-middleware/middlewares/amiusing_middleware.js
@wrongsahil wrongsahil merged commit 592de16 into master May 19, 2026
8 checks passed
@wrongsahil wrongsahil deleted the feat/amiusing-builtin-interception branch May 19, 2026 08:25
@github-actions github-actions Bot mentioned this pull request May 19, 2026
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