feat(WebrtcCamerastreamer): logging and recovery#1731
Conversation
Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the WebRTC camera streamer component to improve code organization and add error resilience for ICE server compatibility issues.
- Extracts inline fetch calls into separate helper methods (
sendInitialRequest,sendRemoteCandidatesRequest,sendOfferRequest,ensureResponseOk) - Implements a fallback mechanism that toggles ICE server inclusion on request failures
- Adds response validation with content-type checking
| await this.ensureResponseOk(response) | ||
| } |
There was a problem hiding this comment.
The ensureResponseOk method consumes the response body by calling response.text() when validation fails. However, for successful responses in sendRemoteCandidatesRequest and sendOfferRequest, the response body is never consumed, which may cause resource leaks or prevent connection reuse in some browsers. Consider consuming the response in the success case as well, e.g., await response.text() after ensureResponseOk, or modify ensureResponseOk to always consume the body.
| await this.ensureResponseOk(response) | ||
| } |
There was a problem hiding this comment.
The ensureResponseOk method consumes the response body by calling response.text() when validation fails. However, for successful responses in sendRemoteCandidatesRequest and sendOfferRequest, the response body is never consumed, which may cause resource leaks or prevent connection reuse in some browsers. Consider consuming the response in the success case as well, e.g., await response.text() after ensureResponseOk, or modify ensureResponseOk to always consume the body.
There was a problem hiding this comment.
A fair point, but AFAIK we can indeed leave to the browser to collect unreferenced responses and dispose of them, so that should be fine to leave as it currently stands.
Improves error handling and logging on the WebRTC Camera Streamer handler, and also includes code that will switch whether to send iceServers or not on retry.