-
Notifications
You must be signed in to change notification settings - Fork 308
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
Restore some of the @icon
checks until resolved
#1325
Conversation
* Do some of the checks that don't pertain to TLS issue * Fix with additional `on('error',...` for the request instead of response. My bad although still doesn't appear to work with native `https.get` and `https.request`. Applies to OpenUserJS#1323
Deployed. |
} else { | ||
fn = /^http:/.test(icon) ? http : https; | ||
|
||
// Workaround for #1323 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping https here until resolved.
}).on('error', function (aErr) { | ||
aInnerCallback(aErr); | ||
}); | ||
}).on('error', function (aErr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added missing request error trap here.
} else { | ||
aInnerCallback(null); | ||
} | ||
}).on('error', function (aErr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial error trap was on response and not the request... still present.
on('error',...
for the request instead of response. My bad although still doesn't appear to work with nativehttps.get
andhttps.request
.Applies to #1323