fix(utils): Streamline IP capturing on incoming requests #13272
Conversation
mydea
requested review from
onurtemizkan,
lforst,
Lms24,
AbhiPrasad and
andreiborza
size-limit report 📦
|
I don't like this change a ton because a) it is a bit intransparent to debug b) it is behaviourally breaking if people set |
I would say that it was a bug that we did not do that before. If I do not enable IP capturing, we should not capture IPs, and it was not a conscious choice to do that (and it's also inconsistent with other platforms, e.g. in python we do what this PR introduces). We do the same for the cookie header, I do not think it is confusing that there is no cookie header if you do not enable cookie capturing, and the same applies here IMHO! If you used to rely on these headers, and suddenly they are gone, and you notice that, you can properly opt-in to IP capturing to get your desired outcome again! |
| const extractedRequestData = Array.isArray(include.request) | ||
| ? extractRequestData(req, { include: include.request }) | ||
| : extractRequestData(req); | ||
| const includeRequest = Array.isArray(include.request) ? [...include.request] : [...DEFAULT_REQUEST_INCLUDES]; |
There was a problem hiding this comment.
q: do we need to spread include.request here?
There was a problem hiding this comment.
yeah, because we mutate this below possibly (adding the ip), and don't want to mutate what the user passes in! (ran into this in a test...)
This PR does three things:
Closes #13260