Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuring HTTP proxy as described in our docs no longer works in Node@18 and @20 #1646

Open
1 task done
waldekmastykarz opened this issue Mar 11, 2024 · 3 comments
Open
1 task done

Comments

@waldekmastykarz
Copy link

Bug Report

Description

Since v18, Node ships with a native fetch client. This client requires a different way to specify an HTTP proxy than what we've been supporting in our SDKs so far.

To specify an HTTP proxy for Node's fetch, you need to use the following code (tested on Node v18.19 and v20.11):

import { ProxyAgent } from 'undici';
import { Client } from '@microsoft/microsoft-graph-client';

const dispatcher = new ProxyAgent(process.env.https_proxy);
const fetchOptions = {
  dispatcher
};

Client.initWithMiddleware({
  middleware,
  fetchOptions
});

What's noteworthy:

  • we specify HTTP proxy using dispatcher instead of agent (agent is ignored on requests, which makes it hard to debug)
  • the ProxyAgent dispatcher comes from undici which seems to be maintained by Node folks. The ProxyAgent is incompatible with other agents that we used to use in the past.

We should update our FetchOptions interface to support specifying dispatcher.

Console Errors: no

Steps to Reproduce

  1. Configure an http proxy as specified in our docs

Expected behavior: web requests intercepted by the specified proxy

Actual behavior: web requests passed through without being intercepted

Additional Context

Usage Information

SDK Version - 3.06

  • Node (Check, if using Node version of SDK)

Node Version - v18.19 and v20.11

@sebastienlevert
Copy link
Contributor

Thanks for the report! At this point, core enhancements won't be added to this library by the team. A PR would be reviewed though.

Also, could this be fixed with just docs? Very suboptimal, but could be an option.

Thoughts?

@waldekmastykarz
Copy link
Author

As far as I can tell, the only thing we need to change in the code is the type that defines fetch options, so that it includes the new dispatcher property. Other than that, we need to update docs and explain how to use it. The code itself works just fine.

@feedmypixel
Copy link

feedmypixel commented Jul 1, 2024

Came across this problem on Node v20.11.1 we were using something similar to - https://learn.microsoft.com/en-us/graph/sdks/customize-client?tabs=typescript#configuring-the-http-proxy-for-the-client:

// Create a client with the proxy
const graphClient = Client.initWithMiddleware({
  authProvider: authProvider,
  fetchOptions: {
    agent: proxyAgent,
  },
});

This was giving us a fetch failed error.

After reverse engineering things and reading through various posts we arrived at nodejs/undici#1350 (comment)

As we are Js and not Ts, we have ended up with:

const graphClient = Client.initWithMiddleware({
  debugLogging: true,
  authProvider,
  fetchOptions: {
    dispatcher: new ProxyAgent({
      uri: proxyUrl,
      keepAliveTimeout: 10,
      keepAliveMaxTimeout: 10
    })
  }
})

Thought this might help someone in the future.

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

No branches or pull requests

3 participants