fix: remove custom proxy handling

[parkerbxyz]

Undici has added native support for proxy handling, so it is no longer necessary for us to have our own custom proxy handling.

Reverts #102 and resolves #134.

[parkerbxyz]

👋 @dmitrijrub @mikakesan Since you helped us test proxy functionality (#99) when we added support for it in #102, would you be willing to help us test this branch (134-remove-custom-proxy-functionality) to ensure you're still able to use this action with a proxy? 🙏

[dmitrijrub]

hi @parkerbxyz, what are the changes? should I just use uses: actions/create-github-app-token@134-remove-custom-proxy-functionality ?

[parkerbxyz]

should I just use uses: actions/create-github-app-token@134-remove-custom-proxy-functionality ?

Yes, exactly! 🙂

[dmitrijrub]

hmmm, looks like its failing

Run actions/create-github-app-token@134-remove-custom-proxy-functionality
owner and repositories not set, creating token for the current repository ("dt-arc")
Failed to create token for "dt-arc" (attempt 1): Connect Timeout Error
Failed to create token for "dt-arc" (attempt 2): Connect Timeout Error
Failed to create token for "dt-arc" (attempt 3): Connect Timeout Error
Failed to create token for "dt-arc" (attempt 4): Connect Timeout Error
RequestError [HttpError]: Connect Timeout Error
    at /home/runner/_work/_actions/actions/create-github-app-token/134-remove-custom-proxy-functionality/dist/main.cjs:3[7](https://github.com/company-company/dt-arc/actions/runs/9512488741/job/26317660736#step:10:8)097:11
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async hook4 (/home/runner/_work/_actions/actions/create-github-app-token/134-remove-custom-proxy-functionality/dist/main.cjs:39469:1[8](https://github.com/org-company/dt-arc/actions/runs/9512488741/job/26317660736#step:10:9))
    at async getTokenFromRepository (/home/runner/_work/_actions/actions/create-github-app-token/134-remove-custom-proxy-functionality/dist/main.cjs:3[9](https://github.com/telia-company/dt-arc/actions/runs/9512488741/job/26317660736#step:10:10)783:20)
    at async RetryOperation._fn (/home/runner/_work/_actions/actions/create-github-app-token/134-remove-custom-proxy-functionality/dist/main.cjs:39660:24) {
  status: 500,
  request: {
    method: 'GET',
    url: xxxx
    headers: {
      accept: 'application/vnd.github.v3+json',
      'user-agent': 'actions/create-github-app-token',
      authorization: 'bearer [REDACTED]'
    },
    request: {
      fetch: [AsyncFunction: fetch],
      hook: [Function: bound hook4] AsyncFunction
    }
  },
  response: undefined,
  attemptNumber: 4,
  retriesLeft: 0
}
Error: Connect Timeout Error

[parkerbxyz]

Noting this in case it is helpful later: nodejs/undici#1650 (comment)

[parkerbxyz]

hmmm, looks like its failing

Thank you so much for testing that, @dmitrijrub! I've made some changes in hopes of fixing the failure you encountered. Would you mind testing again and letting us know how it goes, please? 🙏

[gr2m]

I wonder if we need to explicitly use EnvHttpProxyAgent?
https://github.com/nodejs/undici/blob/7f635e51f6170f4b59abedc7cb45e6bcda7f056d/docs/docs/api/EnvHttpProxyAgent.md#L4

See this comment: nodejs/undici#1650 (comment)

I just wanted to leave that not for when we have another look at it, I don't have time to dig into it now

[parkerbxyz]

I tried that in 5cb4b2f, but several tests started failing as a result. After reading through nodejs/undici#2994, I got the impression it was unnecessary to do this.

Added EnvHttpProxyAgent and made it the default global dispatcher.

Automatically detect/support HTTP_PROXY, HTTPS_PROXY, & NO_PROXY

[bruce-c-liu]

Super random drive-by comment. I got here from that issue. 😅

According to this commit, they ended up not using this as the default agent:
nodejs/undici@f31d3f9

Reasoning:
nodejs/undici#2994 (comment)

So it appears the PR description is wrong. Looks like they'll consider making it the default in a major release.

For now, I was able to get HTTPS_PROXY env variable to work via:

import { EnvHttpProxyAgent, setGlobalDispatcher } from "undici";

setGlobalDispatcher(new EnvHttpProxyAgent());

[parkerbxyz]

Thanks, @bruce-c-liu! I found some additional context here:

https://github.com/nodejs/TSC/blob/41602b95211f30107696a007e5da45b9e933536c/meetings/2024-09-04.md?plain=1#L88-L115

[gr2m]

I don't think this will do anything. There is. no dispatch option in options.request, see all implemented options in https://github.com/octokit/request.js#request

What we have to do is to implement a fetch wrapper

function fetchWithProxyAgent(url, options) {
  return fetch(url, { ... options, dispatcher: envHttpProxyAgent })
}

And then use pass that custom fetch implementation as options.request.fetch

Suggested change

	dispatcher: envHttpProxyAgent,
	fetch: fetchWithProxyAgent,

[parkerbxyz]

Tests are currently failing due to our current use of the global dispatcher in our tests and local dispatcher in the fetch method. The problem can be reproduced with the following example:

import { EnvHttpProxyAgent, MockAgent, setGlobalDispatcher } from "undici";

const mockAgent = new MockAgent();
mockAgent.disableNetConnect();
setGlobalDispatcher(mockAgent);
const mockPool = mockAgent.get("https://example.test");

mockPool
  .intercept({
    path: `/`,
    method: "GET",
  })
  .reply(200);

await fetch("https://example.test", {
  // Comment out the following line to see the test succeed
  dispatcher: new EnvHttpProxyAgent(),
});

console.log("OK");

The global dispatcher is not being used when a local dispatcher is set. We need to verify if this is a feature or a bug.

[parkerbxyz]

The global dispatcher is not being used when a local dispatcher is set. We need to verify if this is a feature or a bug.

nodejs/undici#3792

[parkerbxyz]

Deno appears to have native proxy support: https://docs.deno.com/runtime/reference/env_variables/#special-environment-variables

[gr2m]

Deno appears to have native proxy support

I'm down for using Deno instead of Node.js, as long as we still generate the built files with all dependencies and execute that code instead of loading dependencies dynamically at run time.

[peter-evans]

I just received a really helpful PR here: peter-evans/create-pull-request#3475

And that lead me to realise that proxy support can be simplified to this: peter-evans/create-pull-request#3483

It passes the proxy support tests I have, so looks good.

Just thought I would mention this in case it helps here.