Skip to content

Fix undici proxy gap#375

Merged
brucetony merged 5 commits into
developfrom
fix-undici-proxy-gap
May 22, 2026
Merged

Fix undici proxy gap#375
brucetony merged 5 commits into
developfrom
fix-undici-proxy-gap

Conversation

@brucetony
Copy link
Copy Markdown
Collaborator

@brucetony brucetony commented May 22, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Modified container log retrieval to adjust initial data fetching behavior.
  • Chores

    • Added proxy configuration for authentication requests.
    • Added new dependency to support network proxy functionality.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

Warning

Rate limit exceeded

@brucetony has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 18 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d1cdb61e-5551-4a40-a290-100ebaef67e6

📥 Commits

Reviewing files that changed from the base of the PR and between f11f069 and e3dc739.

📒 Files selected for processing (1)
  • app/services/hub_adapter_swagger.json
📝 Walkthrough

Walkthrough

This PR adds HTTP proxy routing support for outgoing OIDC and NextAuth requests by integrating the node-fetch-native library, while separately simplifying container log fetch parameters by removing explicit unlimited-limit options from initial queries.

Changes

HTTP Proxy Architecture and Log Fetch

Layer / File(s) Summary
HTTP Proxy Dependency and Configuration
package.json, nuxt.config.ts
Adds node-fetch-native@^1.6.7 as a runtime dependency. Nuxt config now computes projectRoot using fileURLToPath from Node.js URL utilities and configures a nitro.alias mapping to route node-fetch-native/proxy to the local node_modules path.
OIDC and NextAuth Request Proxying
server/routes/flame/api/auth/[...].ts
Imports createProxy() from node-fetch-native and integrates proxy configuration into the OIDC discovery fetch, token refresh POST, and NextAuth event handler POSTs by spreading the proxy options into request parameters. Adds explicit TypeScript parameter types for the JWT and session callbacks.
Container Log Fetch Limit Removal
app/components/analysis/logs/ContainerLogs.vue
Removes { limit: null } from the initial getAnalysisLogs() call and the first-fetch branch of refreshLogs(), simplifying request parameters when fetching logs for the first time.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • PrivateAIM/node-ui#361: Both PRs directly modify app/components/analysis/logs/ContainerLogs.vue and the initial/refresh log-fetch flow (including getAnalysisLogs usage and request behavior), so their changes are code-level related around the same log retrieval logic.
  • PrivateAIM/node-ui#364: Both PRs modify app/components/analysis/logs/ContainerLogs.vue's initial and incremental log-fetching logic (including getAnalysisLogs/refreshLogs request parameters such as limit/query), so they are directly related.

Poem

🐰 A proxy hops through the network lanes,
Catching requests on its express trains,
And logs now flow without the chains,
Limits removed, the data reigns!
Clean as lettuce, smooth as grains.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The title 'Fix undici proxy gap' is vague and lacks specificity about what the actual changes accomplish or which components are affected. Consider a more descriptive title that explains the specific issue being fixed, such as 'Add node-fetch-native proxy support for OIDC requests' or 'Configure proxy for authentication HTTP requests'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-undici-proxy-gap

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.

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

🧹 Nitpick comments (1)
nuxt.config.ts (1)

76-80: ⚡ Quick win

Resolve the proxy alias via module resolution (avoid dist/proxy.cjs).

node-fetch-native/proxy is the supported public entry point; dist/proxy.cjs is an internal build artifact, so hardcoding it is brittle. Switch to require.resolve("node-fetch-native/proxy") for stable resolution.

Proposed change
 import { fileURLToPath } from "node:url";
 import { join } from "node:path";
+import { createRequire } from "node:module";

 const projectRoot = fileURLToPath(new URL(".", import.meta.url));
+const require = createRequire(import.meta.url);
+const nodeFetchProxyEntry = require.resolve("node-fetch-native/proxy");

 export default defineNuxtConfig({
@@
   nitro: {
     alias: {
-      "node-fetch-native/proxy": join(projectRoot, "node_modules/node-fetch-native/dist/proxy.cjs"),
+      "node-fetch-native/proxy": nodeFetchProxyEntry,
     },
   },
🤖 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 `@nuxt.config.ts` around lines 76 - 80, The alias currently maps
"node-fetch-native/proxy" to a hardcoded dist path; replace that brittle path
with a module resolution call so the runtime resolves the published entry point.
In the nitro.alias object (symbol: nitro and alias, key
"node-fetch-native/proxy"), change the value from join(projectRoot,
"node_modules/node-fetch-native/dist/proxy.cjs") to use
require.resolve("node-fetch-native/proxy") (or an equivalent module-resolution
call) so the public entry point is resolved instead of the internal dist
artifact.
🤖 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 `@app/components/analysis/logs/ContainerLogs.vue`:
- Line 28: The initial call to getAnalysisLogs in ContainerLogs.vue can be
truncated because it omits limit (API defaults to 1000); update the initial
refreshLogs/fetch path that calls getAnalysisLogs(analysisId) so it explicitly
passes { limit: null } (or equivalent option accepted by getAnalysisLogs) to
request an unbounded snapshot, ensuring lastFetchedAt is set after the full
initial dataset is returned and subsequent polling with query: { start_date:
lastFetchedAt.value } does not permanently miss earlier entries.

In `@server/routes/flame/api/auth/`[...].ts:
- Line 201: The signOut and jwt handler parameter typings are too strict; update
them to match NextAuth shapes (make token/session optional for events.signOut
and make user/account/profile optional for callbacks.jwt) or simply remove the
inline annotations and import/ use NextAuth's official types. For example,
change the signOut signature in signOut to accept { token?: JWT; session?:
Session } and change the jwt callback signature to accept a params object where
token: JWT is required but user?: User | null, account?: Account | null,
profile?: Profile | null, isNewUser?: boolean are optional (or import the
appropriate NextAuth callback types and use those) so the implementations in
signOut and callbacks.jwt align with NextAuth's runtime shapes.

---

Nitpick comments:
In `@nuxt.config.ts`:
- Around line 76-80: The alias currently maps "node-fetch-native/proxy" to a
hardcoded dist path; replace that brittle path with a module resolution call so
the runtime resolves the published entry point. In the nitro.alias object
(symbol: nitro and alias, key "node-fetch-native/proxy"), change the value from
join(projectRoot, "node_modules/node-fetch-native/dist/proxy.cjs") to use
require.resolve("node-fetch-native/proxy") (or an equivalent module-resolution
call) so the public entry point is resolved instead of the internal dist
artifact.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd3f362d-531a-4f10-90d5-f9081a074aba

📥 Commits

Reviewing files that changed from the base of the PR and between f940069 and f11f069.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • app/components/analysis/logs/ContainerLogs.vue
  • nuxt.config.ts
  • package.json
  • server/routes/flame/api/auth/[...].ts

Comment thread app/components/analysis/logs/ContainerLogs.vue
Comment thread server/routes/flame/api/auth/[...].ts
@brucetony brucetony merged commit 640927e into develop May 22, 2026
5 checks passed
@brucetony brucetony deleted the fix-undici-proxy-gap branch May 22, 2026 10:32
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.

1 participant