Skip to content

Conversation

@inancgumus
Copy link
Contributor

@inancgumus inancgumus commented Oct 17, 2025

What?

Fixes a bug when newPageInContext is called without a BrowserContext. Instead of panicking, we now try to use the default browser context if there isn't a current BC, and return an error (as before) if the requested browser ID and the BCs don't match. So, we preserve the current behavior and turn panics into errors.

This fix doesn't solve the underlying race condition issue, though. The next step could be to avoid race conditions between BrowserContext.Close and BrowserContext.NewPage (added #5328).

Why?

When newPageInContext is called without a BrowserContext, we attempt to log with a missing BrowserContext.id, resulting in nil pointer exceptions:

panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x12d0f11]

goroutine 1055774 [running]:
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.deferwrap1()
go.opentelemetry.io/otel/sdk@v1.37.0/trace/span.go:467 +0x25
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc02cda4d20, {0x0, 0x0, 0xc00023ac40?})
go.opentelemetry.io/otel/sdk@v1.37.0/trace/span.go:506 +0xb79
panic({0x1afe4c0?, 0x319c9a0?})
runtime/panic.go:783 +0x132
go.k6.io/k6/internal/js/modules/k6/browser/common.(*Browser).newPageInContext(0xc0002a6000, {0xc00094e280, 0x20})
go.k6.io/k6/internal/js/modules/k6/browser/common/browser.go:457 +0x2f1
go.k6.io/k6/internal/js/modules/k6/browser/common.(*BrowserContext).NewPage(0xc0009bf080)
go.k6.io/k6/internal/js/modules/k6/browser/common/browser_context.go:267 +0x145
go.k6.io/k6/internal/js/modules/k6/browser/browser.mapBrowserContext.func14.1()
go.k6.io/k6/internal/js/modules/k6/browser/browser/browser_context_mapping.go:184 +0x1c
go.k6.io/k6/internal/js/modules/k6/browser/k6ext.promise.func1()
go.k6.io/k6/internal/js/modules/k6/browser/k6ext/promise.go:24 +0x2c
created by go.k6.io/k6/internal/js/modules/k6/browser/k6ext.promise in goroutine 72
go.k6.io/k6/internal/js/modules/k6/browser/k6ext/promise.go:23 +0x9a

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

@inancgumus inancgumus self-assigned this Oct 17, 2025
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 17, 2025 18:13 — with GitHub Actions Inactive
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 17, 2025 18:16 — with GitHub Actions Inactive
@inancgumus inancgumus force-pushed the fix/browser-newpageincontext-bug branch from 7671e04 to 38c72ed Compare October 20, 2025 20:17
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 20, 2025 20:23 — with GitHub Actions Inactive
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 20, 2025 20:25 — with GitHub Actions Inactive
@inancgumus inancgumus force-pushed the fix/browser-newpageincontext-bug branch from 38c72ed to 8229d8c Compare October 21, 2025 16:51
@inancgumus inancgumus changed the title Fix browser.newpageincontext bug Fix browser.newPageInContext bug Oct 21, 2025
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 21, 2025 16:57 — with GitHub Actions Inactive
@inancgumus inancgumus temporarily deployed to azure-trusted-signing October 21, 2025 16:59 — with GitHub Actions Inactive
@inancgumus inancgumus added this to the v1.4.0 milestone Oct 21, 2025
@inancgumus inancgumus marked this pull request as ready for review October 21, 2025 17:05
@inancgumus inancgumus requested a review from a team as a code owner October 21, 2025 17:05
@inancgumus inancgumus requested review from AgnesToulet and ankur22 and removed request for a team October 21, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants