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

Add error catching/logging for recording start and stop/upload problems #402

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

becky-gilbert
Copy link
Contributor

@becky-gilbert becky-gilbert commented Jul 23, 2024

Part of #395 (but should not automatically close that issue).

This PR adds logging about upload problems to the experiment data, so that researchers have some indication of why a video isn't listed in a given response, and improves our Sentry logging for uploading problems.

Explanation of changes

  • Removed the mix of async/await and then/catch promise chains for consistency and to improve error catching. Previously, some promises were awaited but not part of the returned promise chain, so any problems related to those promises were not being caught. Now, all of the necessary steps for starting and stopping/uploading will be in the same promise chain that is returned from the functions, which makes it easer to catch/log errors that occur during the whole chain of events.
  • Added catch blocks to these promise chains, so that we can catch/log all potential problems. The catch blocks in video-recorder should always throw an error so that the error propagates to the video/session recording mixins, where it can be logged in the data. In many cases the video/session recording mixins will also throw an error, but sometimes not (e.g. if the stop recording promise was rejected because of an upload timeout, then we just want to log that event and continue as normal).
  • Improve consistency around rejecting promises (always include a reason) and throwing errors (throw an Error object, not just a string). This will provide more detailed event logs in the data and in Sentry.
  • When a participant leaves the study early, try to complete in-progress trial recording (not just session recording).

New events logged in the data:

  • errorStartingRecording
  • errorStoppingRecorder / errorStoppingSessionRecording
  • recordingUploadTimedout / sessionRecordingUploadTimedout
  • uploadComplete

Examples

Here's what things look like in the experiment data. If this PR is approved then I will add all of this info to the researcher documentation.

Upload successful

If the "uploadComplete" event is logged for a particular file, then that file should be available for the researcher to download. If the researcher does not see the video file in the Responses pages, then they should contact us so that we can investigate.

{
    "videoId": "consent-videoStream_5fc855b5-6d92-4f53-9404-ad251c30bb8f_0-video-consent_a72079fb-99cf-463f-9106-2eea240d0c8c_1721944527372_591",
    "eventType": "exp-lookit-video-consent:uploadComplete",
    "timestamp": "2024-07-25T21:55:39.328Z",
    "streamTime": null
}

Upload timed out

If the "recordingUploadTimedout" event is logged for a particular file, then that file might or might not be available for the researcher to download. If the researcher does not see the video file in the Responses pages after 24 hours, then there was probably a problem with the participant's internet connection during the study session.

{
    "videoId": "consent-videoStream_5fc855b5-6d92-4f53-9404-ad251c30bb8f_0-video-consent_fabe522e-ff4e-42b3-8cc9-1d8c4d8934be_1721945400049_935",
    "eventType": "exp-lookit-video-consent:recordingUploadTimedout",
    "timestamp": "2024-07-25T22:10:12.568Z",
    "streamTime": null
},

Error stopping/uploading

If the "errorStoppingRecorder" event is logged for a particular file, then that file failed to save and will not be available for the researcher to download. This error could be due to problems with our S3 settings/credentials, existing file name in S3 for this upload, in-progress upload that was completed/aborted too soon, etc. This type of error is unlikely to occur but I've included it in the researcher data to allow us to easily identify or rule out missing videos that might be caused by these problems.

{
    "error": "Error: Error: Error completing upload: Error: Upload part failed after 3 attempts.\nError: NoSuchUpload: The specified upload does not exist. The upload ID may be invalid, or the upload may have been aborted or completed.",
    "videoId": "consent-videoStream_5fc855b5-6d92-4f53-9404-ad251c30bb8f_0-video-consent_ff75dbda-0963-404a-a39b-aec576e8b82d_1722034788775_747",
    "eventType": "exp-lookit-video-consent:errorStoppingRecorder",
    "timestamp": "2024-07-26T22:59:57.034Z",
    "streamTime": null
},

@becky-gilbert becky-gilbert self-assigned this Jul 23, 2024
@becky-gilbert becky-gilbert changed the base branch from develop to master July 24, 2024 18:19
@becky-gilbert becky-gilbert changed the base branch from master to develop July 24, 2024 18:20
@becky-gilbert becky-gilbert marked this pull request as ready for review July 26, 2024 23:43
Copy link
Contributor

@bleonar5 bleonar5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this, but it looks great and seems like a really rational improvement. Great work!!

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