Skip to content

Browser: remove undefined headers from route.continue/fulfill #5004

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AgnesToulet
Copy link
Contributor

What?

This PR updates how we parse headers for the route.continue and route.fulfill functions to remove the headers with undefined values.

Why?

Else these headers are sent with "undefined" as a string instead of a null value.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

@AgnesToulet AgnesToulet mentioned this pull request Aug 1, 2025
8 tasks
Base automatically changed from browser/add-route-continue to master August 1, 2025 13:05
@AgnesToulet AgnesToulet force-pushed the browser/route-improve-headers-parsing branch from 8c6e77e to 4a8ae6c Compare August 1, 2025 15:20
@AgnesToulet AgnesToulet marked this pull request as ready for review August 1, 2025 15:22
@AgnesToulet AgnesToulet requested a review from a team as a code owner August 1, 2025 15:22
@AgnesToulet AgnesToulet requested review from ankur22 and codebien and removed request for a team August 1, 2025 15:22
@AgnesToulet AgnesToulet changed the title browser: remove undefined headers from route.continue/fulfill Browser: remove undefined headers from route.continue/fulfill Aug 1, 2025
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Some comments on the speicifics of the implementaitons

result := make([]common.HTTPHeader, 0, len(headersKeys))
for _, hk := range headersKeys {
// Skip undefined headers
if headers.Get(hk).String() == "undefined" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we:

  1. get the value once and use it to check and to go to string below
  2. here check that the value is undefined, not that it gets stringified to "undefined"

@ankur22 ankur22 added this to the v1.3.0 milestone Aug 5, 2025
@ankur22 ankur22 requested a review from mstoykov August 5, 2025 12:33
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.

3 participants