Skip to content

Adjust outgoing activity timestamp#2208

Merged
compulim merged 10 commits intomicrosoft:masterfrom
compulim:fix-1954
Jul 23, 2019
Merged

Adjust outgoing activity timestamp#2208
compulim merged 10 commits intomicrosoft:masterfrom
compulim:fix-1954

Conversation

@compulim
Copy link
Copy Markdown
Contributor

@compulim compulim commented Jul 21, 2019

Fixes #1954.

Changelog Entry

  • Fix #1954. Estimate clock skew and adjust timestamp for outgoing activity, by @compulim in PR #2208

Description

If client clock is skewed significantly behind server clock (client clock < server clock for 5 seconds), the user-originated activity could be appears on top of the transcript momentarily until Direct Line Channel echo back the message with a corrected timestamp.

This is because the timestamp we temporarily assign to the user-originated activity is inaccurate. The insertion-sort put the outgoing activity before other activities received from the server. When server echo back the message, it will correct the timestamp.

This fix is to estimate the clock skew and applies to the outgoing activity.

Specific Changes

  • Modify postActivity.js
    • Piggyback client timestamp to channelData.clientTimestamp, to estimate the clock skew by comparing it with timestamp field
    • When patching timestamp field on outgoing activity, adjust it with the estimated clock skew
  • Add a new reducer, clockSkewAdjustment.js
    • Estimate adjustment on all incoming activity with channelData.clientTimestamp field

  • Testing Added

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 64.88% when pulling b3e04a2 on compulim:fix-1954 into cbee7cb on microsoft:master.

@compulim compulim merged commit 18a9a3b into microsoft:master Jul 23, 2019
@compulim compulim deleted the fix-1954 branch July 23, 2019 07:14
@corinagum
Copy link
Copy Markdown
Contributor

@compulim can we get another PR in with the changes I suggested?

Comment thread packages/core/src/actions/setClockSkewAdjustment.js
Comment thread packages/core/src/sagas/postActivitySaga.js
@compulim compulim mentioned this pull request Jul 26, 2019
corinagum pushed a commit that referenced this pull request Jul 26, 2019
* Fix comments

* Add PR number
@compulim compulim mentioned this pull request Oct 25, 2019
55 tasks
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.

Input message displayed (shortly) above bot message

4 participants