TestBlockIPs data race

[inancgumus]

This job failed with a data race error which you can find below.

=== RUN   TestBlockIPs
==================
WARNING: DATA RACE
Read at 0x00c0030b0218 by goroutine 1559:
  github.com/grafana/xk6-browser/common.(*FrameManager).NavigateFrame()
      /home/runner/work/xk6-browser/xk6-browser/common/frame_manager.go:677 +0x170e
  github.com/grafana/xk6-browser/common.(*Frame).Goto()
      /home/runner/work/xk6-browser/xk6-browser/common/frame.go:843 +0x7d
  github.com/grafana/xk6-browser/common.(*Page).Goto()
      /home/runner/work/xk6-browser/xk6-browser/common/page.go:555 +0x141
  github.com/grafana/xk6-browser/tests.TestBlockIPs()
      /home/runner/work/xk6-browser/xk6-browser/tests/network_manager_test.go:78 +0x2e1
  testing.tRunner()
      /opt/hostedtoolcache/go/1.17.6/x64/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /opt/hostedtoolcache/go/1.17.6/x64/src/testing/testing.go:1306 +0x47

Previous write at 0x00c0030b0218 by goroutine 1661:
  github.com/grafana/xk6-browser/common.(*NetworkManager).onResponseReceived()
      /home/runner/work/xk6-browser/xk6-browser/common/network_manager.go:[567](https://github.com/grafana/xk6-browser/runs/5202862076?check_suite_focus=true#step:8:567) +0x10d
  github.com/grafana/xk6-browser/common.(*NetworkManager).handleEvents()
      /home/runner/work/xk6-browser/xk6-browser/common/network_manager.go:379 +0x2ab
  github.com/grafana/xk6-browser/common.(*NetworkManager).initEvents.func1()
      /home/runner/work/xk6-browser/xk6-browser/common/network_manager.go:354 +0x66

Suspicious code:

xk6-browser/common/network_manager.go

Lines 561 to 569 in 917b83e

	func (m *NetworkManager) onResponseReceived(event *network.EventResponseReceived) {
	req := m.requestFromID(event.RequestID)
	if req == nil {
	return
	}
	resp := NewHTTPResponse(m.ctx, req, event.Response, event.Timestamp)
	req.response = resp
	m.frameManager.requestReceivedResponse(resp)
	}

and FrameManager.NavigateFrame:

xk6-browser/common/frame_manager.go

Lines 674 to 681 in 917b83e

	var resp *Response
	if event.newDocument != nil {
	req := event.newDocument.request
	if req != nil && req.response != nil {
	resp = req.response
	}
	}
	return resp

From the code, it seems like the bug is about concurrently sharing the response object. However, fixing this bug with a lock might not be a good idea, and checking whether requests and responses are nil is kind of an anti-pattern. It's because we dealt with a similar problem before (#188) and fixed the problem by fixing the underlying error instead of using locks. So there might be an underlying issue also here.

---

Maybe related: #271

[inancgumus]

We haven't seen for a long time.