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

Errors installing @ampproject/[email protected] behind proxy #832

Closed
portenez opened this issue Jun 9, 2020 · 9 comments · Fixed by #835
Closed

Errors installing @ampproject/[email protected] behind proxy #832

portenez opened this issue Jun 9, 2020 · 9 comments · Fixed by #835

Comments

@portenez
Copy link

portenez commented Jun 9, 2020

npm install --save @ampproject/[email protected]

node-fetch does not read the http_proxy variables, so there are issues when installing in networks where a proxy is required.

See node-fetch issue about this

@ampproject/[email protected] postinstall /home/suprauser/workspace/wss_wss_nextjs-demo_PR-39/node_modules/@ampproject/toolbox-optimizer

AMP Optimizer ERROR Could not fetch validator rules { FetchError: request to https://cdn.ampproject.org/v0/validator.json failed, reason: connect ETIMEDOUT 172.217.169.33:443
     at ClientRequest.<anonymous> (/home/suprauser/workspace/node_modules/node-fetch/lib/index.js:1455:11)
     at ClientRequest.emit (events.js:189:13)
     at TLSSocket.socketErrorListener (_http_client.js:392:9)
     at TLSSocket.emit (events.js:189:13)
     at emitErrorNT (internal/streams/destroy.js:82:8)
     at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
     at process._tickCallback (internal/process/next_tick.js:63:19)

   message:
    'request to https://cdn.ampproject.org/v0/validator.json failed, reason: connect ETIMEDOUT 172.217.169.33:443',
   type: 'system',
   errno: 'ETIMEDOUT',
   code: 'ETIMEDOUT' }
@sebastianbenz
Copy link
Collaborator

Hm. That's annoying. Not sure what the best way is to fix this. As a workaround you can provide your own fetch implementation:

const nodeFetch = require('node-fetch');

const fetch = (url, opts={}) => {
  opts.agent = new HttpsProxyAgent('proxyHost:proxyPort');
  return nodeFetch(url, opts)
}
const optimizer = AmpOptimizer.create({
  fetch,
});

Let me know if that works and I'll update the docs.

@portenez
Copy link
Author

portenez commented Jun 9, 2020

Yes, the problem is that this runs on postinstall by the package itself. It is not something that my code is running itself. The package actually installs, but I don't know the repercussions of this failing.

@sebastianbenz
Copy link
Collaborator

Got it. The postinstall script failing is no problem. It's just warms up the cache to have a fallback in place in case optimizer is run when offline. We should turn this into a warning instead of an error.

@bernardop
Copy link

We're having the same issue. We're building a Next.js app which has @ampproject/toolbox-optimizer as a dependency. In our case the CI builds are really slow because it hangs trying to fetch the validation rules until it times out (~20 minutes). Is there a way that the postinstall script could look for the rules locally via a path in an env variable, or something like that?

@sebastianbenz
Copy link
Collaborator

Would an aggressive timeout help as well?

@bernardop
Copy link

Yes, that would help a lot in our case.

sebastianbenz added a commit that referenced this issue Jun 10, 2020
sebastianbenz added a commit that referenced this issue Jun 10, 2020
sebastianbenz added a commit that referenced this issue Jun 10, 2020
sebastianbenz added a commit that referenced this issue Jun 10, 2020
@portenez
Copy link
Author

@sebastianbenz When I checked there was no issue opened for nextjs. I bumped into this problem also because of next. I'm wondering if nextjs will need to update the dep in order to get rid of this issue. If that's the case we should open a ticket with nextjs

sebastianbenz added a commit that referenced this issue Jun 11, 2020
sebastianbenz added a commit that referenced this issue Jun 11, 2020
sebastianbenz added a commit that referenced this issue Jun 11, 2020
sebastianbenz added a commit to sebastianbenz/next.js that referenced this issue Jun 11, 2020
This addresses the problem of the optimizer postinstall hook causing
delayed installs if run behind a proxy. See ampproject/amp-toolbox#832

Fixes vercel#14070
@sebastianbenz
Copy link
Collaborator

sebastianbenz commented Jun 11, 2020

@portenez I've created a PR for Next.js (and published the fix to NPM in v2.5.2)

kodiakhq bot pushed a commit to vercel/next.js that referenced this issue Jun 11, 2020
This addresses the problem of the optimizer postinstall hook causing
delayed installs if run behind a proxy. See ampproject/amp-toolbox#832

Fixes #14070
darshkpatel pushed a commit to MLH-Fellowship/next.js that referenced this issue Jun 12, 2020
This addresses the problem of the optimizer postinstall hook causing
delayed installs if run behind a proxy. See ampproject/amp-toolbox#832

Fixes vercel#14070
@mayank-patel
Copy link

Thanks @portenez & @sebastianbenz you guys rock. I had the same trouble and was going to trace back how to fix this.

rokinsky pushed a commit to rokinsky/next.js that referenced this issue Jul 11, 2020
This addresses the problem of the optimizer postinstall hook causing
delayed installs if run behind a proxy. See ampproject/amp-toolbox#832

Fixes vercel#14070
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants