Browser: Don't skip handling local requests #5168
Conversation
| m.logger.Errorf("NetworkManager", "creating request: %s", err) | ||
| return | ||
| } | ||
| // Skip data and blob URLs, since they're internal to the browser. |
There was a problem hiding this comment.
Of course this would break TestURLSkipRequest, as that test is precisely preventing regressions on these lines.
There was a problem hiding this comment.
Arguably this would be a breaking change, so we might need to consider carefully, and perhaps leave it up to k6 v2.0, if we really want to ship it.
| // Skip data and blob URLs, since they're internal to the browser. | ||
| if !isInternalURL(req.url) { | ||
| m.emitRequestMetrics(req) | ||
| } |
There was a problem hiding this comment.
What I don't know, because I lack the historical context from the original PR, is: do we want to skip handling local requests at all, or only to not emit metrics for those requests?
Because with the former, k6 differs from Playwright behavior, in the sense that if a request is supposed to fail, like:
blob:something, for which Chrome (and Playwright) throws net::ERR_FILE_NOT_FOUND at blob:something, k6 doesn't, and the error wouldn't get any error.
While, with the changeset suggested here, the error would be surfaces, as with PW+Chrome, while we would still not emit request metrics for those URLs.
1 participant
What?
Why?
Checklist
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.
Related PR(s)/Issue(s)