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

Add node-fetch custom agent support #51

Closed
wants to merge 1 commit into from

Conversation

mutaphysis
Copy link

@mutaphysis mutaphysis commented Mar 29, 2021

Add node-fetch custom agent support

Description

Adds a new agent property to the options object that
allows configuring the node fetch agent. This can be used
for adding proxy support, doing custom dns resolving and
other advanced use cases.

See https://nodejs.org/api/http.html#http_new_agent_options for
more information on http agents.

Related Issue

#50

Motivation and Context

Will working on a project we had a problem that the back-end we deployed was only allowed network access through a proxy. For our own use of node-fetch this was easily fixed by using a configured https://www.npmjs.com/package/https-proxy-agent but this behaviour was not exposed through the APIs of @adobe/jwt-auth. Thus a simple fix, expose the node-fetch agent property in the API and pass the value through..

How Has This Been Tested?

Tested with a custom proxy aware agent against mitmproxy & a proprietary company proxy. Test cases have not been changed, test coverage is still at 100%, but there is no real test that the agent is actually passed. Typescript types have been extended.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@mutaphysis
Copy link
Author

Is there a way to retrigger travis, seems stuck to me.

@macdonst macdonst closed this Apr 23, 2021
@macdonst macdonst reopened this Apr 23, 2021
@macdonst
Copy link
Contributor

@mutaphysis sorry for not responding for so long. We switched from TravisCI to GitHub Actions and all the tests are now passing. Can you re-base your PR up to the latest code? Also, it would be awesome if you could update the docs as well.

Adds a new `agent` property to the options object that
allows configuring the node fetch agent. This can be used
for adding proxy support, doing custom dns resolving and
other advanced use cases.

See https://nodejs.org/api/http.html#http_new_agent_options for
more information on http agents.
@shazron shazron closed this Jan 18, 2022
@shazron shazron reopened this Jan 18, 2022
@@ -13,6 +13,8 @@ governing permissions and limitations under the License.
// Type definitions for @adobe/jwt-auth 0.3
// Project: https://github.com/adobe/jwt-auth#readme

import { Agent } from 'http'
Copy link
Member

Choose a reason for hiding this comment

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

you can also have a https.Agent

@@ -101,7 +102,8 @@ async function authorize(options) {
const postOptions = {
method: 'POST',
body: form,
headers: form.getHeaders()
headers: form.getHeaders(),
agent
Copy link
Member

Choose a reason for hiding this comment

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

technically node-fetch custom agent can take in a function as well (see node-fetch docs)
Documentation is missing in README.md regarding the agent option.

@mutaphysis mutaphysis closed this Mar 18, 2022
@mutaphysis mutaphysis deleted the feat/http-agent-support branch March 18, 2022 16:02
@mutaphysis mutaphysis restored the feat/http-agent-support branch March 18, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants