Skip to content
This repository has been archived by the owner on Jun 14, 2018. It is now read-only.

Feature/request library #8

Merged
merged 3 commits into from
Oct 5, 2017

Conversation

micahriggan
Copy link

@micahriggan micahriggan commented Oct 5, 2017

The native nodejs https.request method doesn't follow proxy environment variables.
This is referenced by nodejs/node#8381

The request library does support these environment variables.

These environment variables are commonly used at the corporation I work for, so not supporting them is a barrier to entry for many corporate developers.

This should close
trufflesuite/truffle-init#1

@micahriggan micahriggan changed the base branch from master to develop October 5, 2017 16:22
@micahriggan
Copy link
Author

Master branch tests fail on work machine with

  1. Unbox unboxes truffle box from github:
    Uncaught Error: getaddrinfo ENOTFOUND raw.githubusercontent.com raw.githubusercontent.com:443
    at errnoException (dns.js:28:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:76:26)

After this PR

Unbox
✓ unboxes truffle box from github (1225ms)
✓ ignores files listed in the truffle-init.json file, and removes the truffle-init.json file
✓ won't re-init if truffle.js file exists

3 passing (1s)

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@tcoulter could you also review to test on Windows?

@gnidan gnidan requested a review from tcoulter October 5, 2017 16:41
Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Sorry - missed something. Mind making that addition? Then it should be good to merge.

};
var req = https.request(options, function(r) {
request(options, function(error, r) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@micahriggan actually one comment - would you mind adding a check for error now that it's a new arg?

Copy link
Author

Choose a reason for hiding this comment

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

~       if (error) {
+         return reject(new Error("Error making request. Please check the format of the requested resource: " + options.uri));
+       } else if (r.statusCode == 404) {

Would something like this work?

@gnidan gnidan merged commit e4be03b into trufflesuite:develop Oct 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants