Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RUMF-1147] 🐛 Implement TextEncoder().encode fallback for replay encorder #1269

Merged
merged 6 commits into from
Jan 18, 2022

Conversation

amortemousque
Copy link
Contributor

Motivation

RUM session replay on Edge 18 when encoding records to UTF-8 an error is thrown.
This PR implement a fallback if TextEncoder().encode is not supported.

Changes

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@amortemousque amortemousque requested review from a team as code owners January 14, 2022 08:09
@amortemousque
Copy link
Contributor Author

amortemousque commented Jan 14, 2022

Talking with @BenoitZugmeyer, I tried two different approaches for the fallback:

  1. using FileReader:
    It introduce an asynchronous behaviour inside the web worker and messages aren't guaranteed to be treated in order.

  2. using pako encoding function
    It is synchronous but use a custom encoding function adding a bit more code

IMO, In term of complexity Pako fallback is the best solution.

Additional question:
By looking at this test I'm wondering , if it is possible to enable session replay on IE 11 or is there other blockers?

@codecov-commenter
Copy link

Codecov Report

Merging #1269 (007528e) into main (c36718f) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1269      +/-   ##
==========================================
- Coverage   89.93%   89.90%   -0.03%     
==========================================
  Files         105      105              
  Lines        4370     4370              
  Branches      989      989              
==========================================
- Hits         3930     3929       -1     
- Misses        440      441       +1     
Impacted Files Coverage Δ
packages/core/src/tools/timeUtils.ts 97.14% <0.00%> (-2.86%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c36718f...007528e. Read the comment docs.

@amortemousque amortemousque merged commit ab575e6 into main Jan 18, 2022
@amortemousque amortemousque deleted the aymeric/textencoder-is-not-available-on-edge-18 branch January 18, 2022 09:00
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.

4 participants