Add Proxy for Fetch in download.ts - Resolves issue 1280 #1291

cdesch · 2021-03-02T00:16:36Z

Added functionality to allow HTTP_PROXY and HTTPS_PROXY environment variables for proxy settings.

This addresses the issue#1280 to be able to download the compilers behind a corporate firewall via a proxy.

This pull request also adds test cases packages/hardhat-core/test/internal/util/download.ts for the existing functionality without using a proxy and using a proxy.

…PROXY

…are no types for this package

… standard UPPERCASE env convention

cdesch · 2021-03-02T12:07:27Z

I feel that the Windows CI Build is failing across the board as noted with PR #1268

cdesch · 2021-03-03T20:53:35Z

It would be mighty cool if this PR was checked out.... might help some us behind proxies use HardHat instead of the TruffleSuite.

😁

alcuadrado · 2021-03-03T20:55:46Z

Hey @cdesch, thanks for submitting this PR. We'll review it in the next few days or early next week.

cdesch · 2021-03-04T18:55:22Z

Thank you. Surely it will make HardHat a little more accessible :-p

cdesch · 2021-03-15T03:00:57Z

@alcuadrado is there anything I can do to assist with getting this pull request merged?

fvictorio · 2021-03-15T12:09:27Z

Hey @cdesch, sorry for not having reviewed this yet. I'll take a look this week.

cdesch · 2021-03-16T21:37:36Z

@fvictorio Thanks. Let me know if there is anything I can do to help. I'm happy to revise or making another PR if you have feedback. There can be some fun issues that crop up with proxies and it is kind of an edge case for some folks, which is why I included tests. I worked a little bit on adding custom certificates for the proxy but it seems that isn't needed currently.

I used ENV variables in my commit as it is a pretty standard process for most proxies and the same as when using a proxy for npm. hardhat.config.js didn't feel like the right place, but you may have a different opinion about including proxy configs there.

fvictorio · 2021-03-17T14:54:21Z

I used ENV variables in my commit as it is a pretty standard process for most proxies and the same as when using a proxy for npm. hardhat.config.js didn't feel like the right place, but you may have a different opinion about including proxy configs there.

That seems fine to me, there seems to be a convention around those two environment variables and as far as I can tell the right place to handle them in this scenario is in Hardhat (there's an issue about these variables in node-fetch but they decided that it should be done at the application level).

fvictorio · 2021-03-19T20:27:50Z

@cdesch Thanks a lot for this. I made some minor changes. I'll add some comments explaining them in case you are interested 🙂

fvictorio · 2021-03-19T20:28:45Z

packages/hardhat-core/src/internal/util/download.ts


-  const response = await fetch(url, { timeout: timeoutMillis });
+  if (process.env.HTTPS_PROXY !== undefined) {
+    fetchOptions.agent = new HttpsProxyAgent(process.env.HTTPS_PROXY);


There was some unnecessary casting here. The properties of process.env have type string | undefined, so if you check for that in the if, typescript will know that the type is string inside the block.

Thanks! Yeah, I was struggling a bit on the TypeScript learning curve, but I'm slowly migrating a project to TypeScript currently.

fvictorio · 2021-03-19T20:29:11Z

packages/hardhat-core/src/internal/util/download.ts

@@ -1,5 +1,7 @@
 import fs from "fs";
 import fsExtra from "fs-extra";
+import { HttpsProxyAgent } from "https-proxy-agent";
+import type { RequestInit as FetchOptions } from "node-fetch";


Manually defining FetchOptions wasn't necessary, it is better to import it from the module.

Ah. I think I was a bit confused on that because of the const { default: fetch } = await import("node-fetch"); which seemed strange as to where to put the import. Thank you!

fvictorio · 2021-03-19T20:30:05Z

packages/hardhat-core/test/internal/util/download.ts

+      // Setup Proxy Server
+      proxy = new Proxy();
+      connections = 0;
+      proxy.on("connection", () => {


I added this so we can assert that the proxy is really being used.

fvictorio · 2021-03-19T20:32:05Z

packages/hardhat-core/test/internal/util/download.ts

+          process.env.HTTPS_PROXY = oldHttpsProxyEnv;
+        } else {
+          delete process.env.HTTPS_PROXY;
+        }


To save the current environment, you were doing something like this:

env = process.env process.env.HTTP_PROXY = ...

But since process.env is an object, any modifications you were doing to it were also done to env, so nothing was restored afterwards.

I also have to explicitly use delete here because it turns out that

process.env.FOO = undefined

doesn't remove the FOO environment variable. Instead, it sets it to the string "undefined". Terrible behavior from node, but it is what it is.

Nice catch. I think adapted that portion from the https-proxy-agent package, and in fact I wasn't sure if the env really needed to be changed back at the end of the test because I wasn't sure if changes to the process.env were scoped to the test or not. It seems like they are.

Thanks for catching this.

cdesch · 2021-03-19T21:27:42Z

Thank you @fvictorio for the comments and the feedback! I learned a lot in the process to get this working.

cdesch · 2021-03-26T14:18:12Z

@fvictorio Do you think this PR will make the next HH release? Possibly 2.2.x or minor like 2.1.3?

Would you like me to add docs for this?

alcuadrado · 2021-03-30T15:47:42Z

Hey @cdesch, thanks for this PR. Unfortunately we have to focus on implementing Berlin right now, so we won't get merged this in the next couple of weeks. Once the new hardfork support is out, we'll review and release this.

cdesch · 2021-04-02T13:56:52Z

Thanks @alcuadrado, Can you recommend how I can install the NPM package from github in the meantime? I understand that typically you can do a npm install https://github.com/<username>/<repository> but since the HardHat repo has multiple packages in it under packages/* such as packages/hardhat-core, etc... How would I install just one or more of them manually until the next release?

My apologies for being persistent, this issue is a show stopper for my team using hardhat on a new project, but we understand that you have priorities too.

alcuadrado · 2021-04-02T16:05:41Z

I'm afraid it can't be installed like that. Check the contributing.md which explains how to build it locally.

cdesch · 2021-04-14T03:59:57Z

@alcuadrado Okay. Thank you.

cdesch · 2021-05-02T22:46:57Z

@alcuadrado and @fvictorio Happy Weekend! Any chance of looking at this PR this week? 😇

cdesch · 2021-05-04T14:50:01Z

I updated this PR to the current Master branch and attempted to resolve the merge conflict with yarn.lock.

The test suite fails with:

  1 failing

  1) Eth module
       JSON-RPC provider
         eth_sendTransaction
           return txHash
             Should return the hash of a reverted transaction:
     TypeError: Cannot read property 'txHash' of undefined
      at Context.<anonymous> (test/internal/hardhat-network/provider/modules/eth.ts:4005:39)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:94:5)

but that seems to be from 0a575a38a and PR 1425, although I could be wrong. The error does not appear to be related to this PR.

cdesch · 2021-05-16T19:14:07Z

Checking in to see if this can be merged in the next version. Is there anything I can do to help merge this in?

alcuadrado · 2021-05-18T22:50:44Z

Hey @cdesch! I'm really sorry I took so long to review this. Thanks for submitting this PR, and for being patient.

As I read that you were struggling a bit with ts, I pushed this commit improving a few tiny bits, which you may find interesting.

I also added some docs about this. I wasn't sure where to place this info, so I added a warning box to the compilation guide for now. What do you think, @fvictorio?

cdesch

Thank you

cdesch · 2021-05-19T13:29:25Z

Thank you @alcuadrado! I think everything looks good. I will test as soon as the package is published.

alcuadrado · 2021-05-19T21:21:36Z

@fvictorio if you think the docs change is ok we can merge this.

fvictorio · 2021-05-20T00:12:03Z

Thanks @cdesch and sorry for the huge delay!

cdesch · 2021-05-20T16:18:19Z

No worries, I completely understand! Thank you @fvictorio ! I'm ready to test it out!

hygkui · 2022-05-02T14:00:26Z

npx hardhat --show-stack-traces --verbose compile can show the download url, you can download and move it to the right dir such like '$HOME/Library/Caches/hardhat-nodejs/compilers/macosx-amd64/solc-macosx-amd64-v0.x.x+commit.fc410830', and it can be sloved the compiler download issue.

cdesch added 10 commits March 1, 2021 17:42

Adding proxy for download.ts and tests for download.tx

ffdbfbc

Adding ENV variables HTTP_PROXY and HTTPS_PROXY

e747715

remove unused function

ed32fe8

Fixing TypeScript Linting Errors. Adding test specifically for HTTPS_…

3b0464d

…PROXY

Adding @ts-ignore to suppres Types warning on 'proxy' package. There …

9e46ee5

…are no types for this package

Fixing import lint error and rearranging imports to be alphabetical. OMG

48e030e

removing support for lowercase http_proxy and https_proxy in favor of…

b3b2a13

… standard UPPERCASE env convention

Fixing TypeScript linting errors

a3e9f5e

Fixing linting errors. tslint is very strict.

a7c579e

adding back @ts-ignore. Package 'proxy' does not have typescript types

40d8e2d

fvictorio added the package:hardhat-core label Mar 8, 2021

cdesch mentioned this pull request Mar 16, 2021

Downloading SOLC behind proxy - HardhatError: HH502: Couldn't download compiler versions list. Please check your connection. #1280

Closed

fvictorio force-pushed the master branch from 25d78e0 to 837ac65 Compare March 19, 2021 19:17

fvictorio reviewed Mar 19, 2021

View reviewed changes

cdesch force-pushed the master branch from 02256c0 to 40d8e2d Compare May 4, 2021 14:36

Merge branch 'master' into master

11d3c57

alcuadrado added 3 commits May 18, 2021 19:29

Merge branch 'master' into cdesch/master

2cb0cb3

Small ts improvements

1083492

Add doc about the env variables

eb07d12

alcuadrado approved these changes May 18, 2021

View reviewed changes

cdesch commented May 19, 2021

View reviewed changes

fvictorio merged commit 677099d into NomicFoundation:master May 20, 2021

furkilic mentioned this pull request Oct 22, 2021

Add the ability to deploy to networks behind a Corporate HttpProxy #1978

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Proxy for Fetch in download.ts - Resolves issue 1280 #1291

Add Proxy for Fetch in download.ts - Resolves issue 1280 #1291

cdesch commented Mar 2, 2021

cdesch commented Mar 2, 2021

cdesch commented Mar 3, 2021

alcuadrado commented Mar 3, 2021

cdesch commented Mar 4, 2021

cdesch commented Mar 15, 2021

fvictorio commented Mar 15, 2021

cdesch commented Mar 16, 2021

fvictorio commented Mar 17, 2021

fvictorio commented Mar 19, 2021

fvictorio Mar 19, 2021

cdesch Mar 19, 2021

fvictorio Mar 19, 2021

cdesch Mar 19, 2021

fvictorio Mar 19, 2021

fvictorio Mar 19, 2021

cdesch Mar 19, 2021

cdesch commented Mar 19, 2021

cdesch commented Mar 26, 2021

alcuadrado commented Mar 30, 2021

cdesch commented Apr 2, 2021

alcuadrado commented Apr 2, 2021

cdesch commented Apr 14, 2021

cdesch commented May 2, 2021

cdesch commented May 4, 2021

cdesch commented May 16, 2021

alcuadrado commented May 18, 2021

cdesch left a comment

cdesch commented May 19, 2021

alcuadrado commented May 19, 2021

fvictorio commented May 20, 2021

cdesch commented May 20, 2021

hygkui commented May 2, 2022

Add Proxy for Fetch in download.ts - Resolves issue 1280 #1291

Add Proxy for Fetch in download.ts - Resolves issue 1280 #1291

Conversation

cdesch commented Mar 2, 2021

cdesch commented Mar 2, 2021

cdesch commented Mar 3, 2021

😁

alcuadrado commented Mar 3, 2021

cdesch commented Mar 4, 2021

cdesch commented Mar 15, 2021

fvictorio commented Mar 15, 2021

cdesch commented Mar 16, 2021

fvictorio commented Mar 17, 2021

fvictorio commented Mar 19, 2021

fvictorio Mar 19, 2021

Choose a reason for hiding this comment

cdesch Mar 19, 2021

Choose a reason for hiding this comment

fvictorio Mar 19, 2021

Choose a reason for hiding this comment

cdesch Mar 19, 2021

Choose a reason for hiding this comment

fvictorio Mar 19, 2021

Choose a reason for hiding this comment

fvictorio Mar 19, 2021

Choose a reason for hiding this comment

cdesch Mar 19, 2021

Choose a reason for hiding this comment

cdesch commented Mar 19, 2021

cdesch commented Mar 26, 2021

alcuadrado commented Mar 30, 2021

cdesch commented Apr 2, 2021

alcuadrado commented Apr 2, 2021

cdesch commented Apr 14, 2021

cdesch commented May 2, 2021

cdesch commented May 4, 2021

cdesch commented May 16, 2021

alcuadrado commented May 18, 2021

cdesch left a comment

Choose a reason for hiding this comment

cdesch commented May 19, 2021

alcuadrado commented May 19, 2021

fvictorio commented May 20, 2021

cdesch commented May 20, 2021

hygkui commented May 2, 2022