Session Replay Transport for Rollbar.js #1182
Conversation
- Update Queue._handleReplayResponse to return meaningful Promises - Replace setTimeout in tests with async/await for deterministic behavior - Fix replayMap test to properly mock context management and span export queue - Update test expectations to match modern implementation This change maintains backward compatibility while making tests more deterministic and less flaky by using Promises instead of arbitrary timeouts.
- Update _postPromise to use object destructuring for labeled parameters - Replace if/else in callback with ternary operator for more concise code - Maintain backward compatibility with existing code - All tests passing (unit and integration)
| navigator: { | ||
| userAgent: 'Mozilla/5.0 Test', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The tests run in the context of a headless chrome instance, and usually tests that interact with window state are better using the real browser window unless there's a specific reason to mock.
Example:
https://github.com/rollbar/rollbar.js/blob/master/test/tracing/tracing.test.js#L35
There was a problem hiding this comment.
Got it, will look into it, thanks.
| } | ||
|
|
||
| recordingSpan.setAttribute('rollbar.replay.id', replayId); | ||
| this.#exporter.export([recordingSpan.span]); |
There was a problem hiding this comment.
When recorder.dump() calls span.end(), this is already done. https://github.com/rollbar/rollbar.js/blob/master/src/tracing/spanProcessor.js#L11-L14
There was a problem hiding this comment.
The doc refers to exporter.export here. That's my mistake.
There was a problem hiding this comment.
Right, because the act of ending the span makes the span processor export the span right? Oversight.
Edit: corrections cause on phone
There was a problem hiding this comment.
ending the span makes the span processor export the span right?
👍 Yes
|
|
||
| recordingSpan.setAttribute('rollbar.replay.id', replayId); | ||
| this.#exporter.export([recordingSpan.span]); | ||
| const spans = spanExportQueue.slice(); |
There was a problem hiding this comment.
The exporter needs a method to map spans in the queue to a payload and return that payload.
There was a problem hiding this comment.
Yes! This is something we need to talk about because I don't fully grasp it, yet.
This is because we don't export the ReadableSpan like we said yesterday, right? We may it into our API payload.
Edit: phone messes up
Description of the change
Description by Claude
Goal
Implement a robust transport mechanism for session replay data in the Rollbar.js SDK by:
Key Components
ReplayMap
Queue Integration
API Extensions
Major Changes
Code Improvements
- Updated Queue._handleReplayResponse to return meaningful Promises
- Improved error handling with explicit return values
- Maintained backward compatibility with existing code
- Fixed context access pattern (contextManager.active() vs context().active())
- Improved span export mechanism to properly integrate with spanExportQueue
- Added robust error handling throughout
Testing Infrastructure
- Separated unit and integration tests into dedicated directories
- Created proper index files for clean imports
- Added dedicated test commands for each test type
- Replaced flaky setTimeout-based assertions with async/await
- Implemented spies on Promise-returning methods to await actual completion
- Created more deterministic test fixtures
- Updated MockTracing to match current implementation
- Improved SpanExporter mock to better simulate actual behavior
- Created precise assertions that verify exact data passed between components
Integration Flow
Benefits of Implementation
This implementation ensures session replay data is correctly associated with errors and reliably transmitted to the Rollbar backend, providing crucial context for debugging production issues.
Type of change
Related issues
Checklists
Development