Skip to content

Handle undici module availability more robustly - #102

Open
dbigham wants to merge 1 commit into
mainfrom
codex/fix-module-not-found-error-for-undici
Open

Handle undici module availability more robustly#102
dbigham wants to merge 1 commit into
mainfrom
codex/fix-module-not-found-error-for-undici

Conversation

@dbigham

@dbigham dbigham commented Oct 12, 2025

Copy link
Copy Markdown
Owner

Summary

  • allow the server to prefer Node.js's built-in undici implementation when available
  • fall back to the installed undici package and surface a descriptive error if neither can be loaded

Testing

  • npm test (fails: Missing Questrade refresh token(s). Seed token-store.json with at least one login.)

https://chatgpt.com/codex/tasks/task_e_68ebb8701774832d9460683c4dc96c24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment thread server/src/index.js
Comment on lines +10 to +35
let undiciRequest;
let UndiciAgent;
let UndiciProxyAgent;

try {
({ request: undiciRequest, Agent: UndiciAgent, ProxyAgent: UndiciProxyAgent } = require('node:undici'));
} catch (nodeUndiciError) {
try {
({ request: undiciRequest, Agent: UndiciAgent, ProxyAgent: UndiciProxyAgent } = require('undici'));
} catch (npmUndiciError) {
const combinedError = new Error(
'Failed to load undici. Ensure you are running on Node.js v18+ or install the "undici" package.',
);
combinedError.details = {
nodeUndiciError: {
message: nodeUndiciError instanceof Error ? nodeUndiciError.message : String(nodeUndiciError),
code: nodeUndiciError && typeof nodeUndiciError === 'object' ? nodeUndiciError.code : undefined,
},
npmUndiciError: {
message: npmUndiciError instanceof Error ? npmUndiciError.message : String(npmUndiciError),
code: npmUndiciError && typeof npmUndiciError === 'object' ? npmUndiciError.code : undefined,
},
};
throw combinedError;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard against missing ProxyAgent in built‑in undici

The new loading logic prefers require('node:undici') and only falls back to the npm package if the require itself throws. On many Node 18 LTS builds the built‑in node:undici export does not include ProxyAgent, so the destructuring succeeds but UndiciProxyAgent becomes undefined. When a proxy is configured and getDispatcherForUrl attempts new UndiciProxyAgent({ uri }), the server will now throw TypeError: UndiciProxyAgent is not a constructor at runtime, whereas the previous code always used the npm dependency that ships with ProxyAgent. Consider detecting missing exports and falling back to the npm module when ProxyAgent (or other required symbols) is undefined.

Useful? React with 👍 / 👎.

@dbigham
dbigham force-pushed the main branch 3 times, most recently from c7f2145 to c8636e3 Compare November 15, 2025 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant