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

Cache remoteAddress #86

Open
Cerberooo opened this issue Jul 1, 2019 · 3 comments

Comments

@Cerberooo
Copy link

commented Jul 1, 2019

For sindresorhus/got#827 we need the remoteAddress in the cache. This would require (as I understand correctly) connection: { remoteAddress: response.connection.remoteAddress } in the following lines:

const value = {
cachePolicy: response.cachePolicy.toObject(),
url: response.url,
statusCode: response.fromCache ? revalidate.statusCode : response.statusCode,
body
};

And a connection parameter in https://github.com/lukechilds/responselike/blob/093a21bc25ecc82daf17f436c13fd807e5d8eee0/src/index.js#L7

Is this a conceivable change?

@szmarczak

This comment has been minimized.

Copy link
Collaborator

commented Jul 1, 2019

Remember to set .socket = .connection as these point to the same object. Indeed, it would be very nice to see it here but as you've already pointed out I think we should make some hooks here.

@SleeplessByte

This comment has been minimized.

Copy link

commented Jul 1, 2019

Please don't add it to the lines as listed by @Cerberooo. The Remote IP address is not a field specified by RFC 7234 and we heavily rely, spec compliantly, on this behaviour. IMO, this should indeed be added either via hooks or by layering.

@Cerberooo Cerberooo referenced this issue Aug 18, 2019
4 of 4 tasks complete
@sindresorhus

This comment has been minimized.

Copy link
Collaborator

commented Aug 18, 2019

If we can't change the default, there needs to be a way for us to hook it so that we can add it ourselves. (See sindresorhus/got#827)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.