Skip to content

fix: local server not capturing error events from the browser#152

Merged
zivl merged 1 commit intozivl:masterfrom
rchl:fix/browser-v7
Jan 3, 2023
Merged

fix: local server not capturing error events from the browser#152
zivl merged 1 commit intozivl:masterfrom
rchl:fix/browser-v7

Conversation

@rchl
Copy link
Copy Markdown
Contributor

@rchl rchl commented Dec 19, 2022

Support for capturing error events sent from the Browser (as opposed to from the Node) was missing. When browser makes a POST request to the server, the transfer-encoding: chunked encoding is not used and the block that handled that case was missing error parsing and adding to reports().

While at it, I've consolidated 3 places that did the request body parsing and event processing into single exported function. This should hopefully ensure that no one forgets about handling one of the places in the future as there will be only one place to change. (Let me know if this new function should live somewhere else).

I've looked into adding test for this case but that would require a setup for testing using real browser and that would require quite a bit of work to set up and those tests would significantly slow down the testsuite.

@zivl
Copy link
Copy Markdown
Owner

zivl commented Jan 2, 2023

@rchl I accidentally merged #153 before #152,
can you pls resolved the conflicts so I can merge and release?

@rchl
Copy link
Copy Markdown
Contributor Author

rchl commented Jan 2, 2023

Since #153 was based on this one, by merging #153 you've also merged this one. That means that those two were combined into one commit and the commit is missing some relevant commit message but if that's OK then we can just close this one.

@rchl
Copy link
Copy Markdown
Contributor Author

rchl commented Jan 2, 2023

Alternatively you could revert the #153 merge, merge this first, let me rebase #153 and then merge it.

@zivl
Copy link
Copy Markdown
Owner

zivl commented Jan 3, 2023

@rchl yeah you're right.
let me know if and when you can rebase & merge, or you want me to do it later this week

@rchl
Copy link
Copy Markdown
Contributor Author

rchl commented Jan 3, 2023

I can do it at any time but then you'd need to click revert on #153 first. It should create a revert commit and then you should be able to merge this one first.

@zivl
Copy link
Copy Markdown
Owner

zivl commented Jan 3, 2023

on it #160

@rchl
Copy link
Copy Markdown
Contributor Author

rchl commented Jan 3, 2023

Feel free to merge this one first now.

@zivl zivl merged commit 6e43809 into zivl:master Jan 3, 2023
@rchl rchl deleted the fix/browser-v7 branch January 3, 2023 18:49
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.

2 participants