fix(app, app-shell): Support protocol backwards app compatibility #13518

mjhuff · 2023-09-12T12:41:08Z

Overview

This PR fixes an issue where users who downgrades their app from v7.0.0 to 6.3.1 or lower whitescreen on startup due to protocols supporting modules not present in previous versions. Because a v6.3.1 and 7.0.0 version of the app must both be supported until OT-2/Flex app parity, there's need for a workaround solution.

The Immediate Problem

In order to solve this issue without deploying a hotfix for 6.3.1, protocols for version 7.0+ are stored in a new protocols folder (within the same directory as the old). A migration occurs on startup where new protocols added in the 6.3.1 app are copied over for use within the 7.0.0 app.

Using a copy rather than a simple folder rename prevents users who have both versions of the app from seeing none of their protocols in the v6.3.1 version.

NOTE: Very large protocol files take a while to transfer and may result in a temporary whitescreen on app startup, however, in practice this should not be an issue in practice- only significantly large protocols made with the purpose of testing protocol size will cause noticeable delay.

The Future Problem

To prevent the need for a stop-gap solution in the future, a simple error boundary is added.

Test Plan

Open the app on this branch.
Observe protocol migration by going to /Users/user_name/Library/Application Support/Opentrons/ and opening the protocols and protocols_v7.0-supported directories. Only protocols before the v6.3.1 release date are present in the former directory, and all protocols are present in the latter directory.
Use the protocol below, which is for a module that doesn't exist (manually add it to the new protocols directory). Observe the error message upon app restart.
34564c7z-f57a-4ca6-9e2f-725aedeb1149.zip

Changelog

Added temporary stop-gap to migrate protocols until app parity.
Added protocol error boundary to prevent this issue from occurring in the future.

Risk assessment

low

Downgrading the app from v7.0.0 to v6.3.1 or earlier causes whitescreening when protcols that use new modules are present. This migration is a temporary shim to enable backwards compability until OT-2 and Flex app parity is achieved. Co-authored-by: Seth Foster <[email protected]> Co-authored-by: Jethary Rader <[email protected]>

Render an error message instead of crashing if a user views a protocol with an instrument or module that does not yet exist. This behavior is most likely triggered when a user downgrades their app. Co-authored-by: Seth Foster <[email protected]> Co-authored-by: Jethary Rader <[email protected]>

codecov · 2023-09-12T12:46:48Z

Codecov Report

Merging #13518 (4008bed) into chore_release-7.0.0 (124f184) will increase coverage by 0.00%.
Report is 10 commits behind head on chore_release-7.0.0.
The diff coverage is 58.33%.

❗ Current head 4008bed differs from pull request most recent head 001df5e. Consider uploading reports for the commit 001df5e to get more accurate results

Additional details and impacted files

@@                   Coverage Diff                   @@
##           chore_release-7.0.0   #13518      +/-   ##
=======================================================
  Coverage                71.36%   71.36%              
=======================================================
  Files                     2418     1586     -832     
  Lines                    67937    52751   -15186     
  Branches                  7886     3437    -4449     
=======================================================
- Hits                     48480    37646   -10834     
+ Misses                   17616    14574    -3042     
+ Partials                  1841      531    -1310

Flag	Coverage Δ
app	`44.08% <58.33%> (-24.89%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
app-shell/src/protocol-storage/index.ts	`41.89% <54.54%> (+5.35%)`	⬆️
app-shell/src/protocol-storage/file-system.ts	`78.57% <100.00%> (+0.38%)`	⬆️

... and 833 files with indirect coverage changes

jerader

This is awesome, thank you so much for tackling this!! ⭐ 🚀 🥇 For follow up, let's make a ticket in RAUT and label it as high priority so we don't forget about it. And once this merges, we probably want to make a post in app & ui + 7.0.0 release planning slack channels to let people know that this migration will happen.

jerader · 2023-09-12T12:59:31Z

app-shell/src/protocol-storage/file-system.ts

+// TODO(jh, 2023-09-11): remove OLD_PROTOCOLS_DIRECTORY_PATH after
+// OT-2 parity work is completed and move all protocols back to "protocols" directory.


Let's make a ticket to remind us to remove this after 7.0.0 launch!

Will do - creating a chore ticket now 👍

app/src/organisms/ProtocolsLanding/ProtocolCard.tsx

sfoster1

I think let's get rid of the deletion of protocols - most likely we won't have a chance to remove it before at least a beta build, and we can tell in house people to just delete their app config - and we definitely have to use the promise versions of the fs functions. Otherwise, thanks for tracking this down and finding a fix!

sfoster1 · 2023-09-12T13:57:40Z

app-shell/src/protocol-storage/index.ts

+    }
+  }
+
+  // TODO(jh, 2023-09-11): Remove  deleteIfVersion7Protocol() after


IMO if this is to handle in-house only or even the guided installs we don't need it

sfoster1 · 2023-09-12T14:06:07Z

app-shell/src/protocol-storage/index.ts

+    }
+  }
+  function migrateProtocols(src: string, dest: string): void {
+    if (fse.existsSync(src)) {


i think that since this is called in a promise chain anyway we should be using the promise versions of these functions, so we can run all the migrations concurrently through this forEach and gather with Promise.all. in-person conversation indicates that using the sync variants and blocking the node event loop is what causes the whitescreen for large protoocls so that should fix it.

mjhuff · 2023-09-12T16:55:15Z

Thanks for the feedback @jerader and @sfoster1. Changes are in - everything appears to be working.

koji · 2023-09-12T17:07:51Z

Let me test this branch on Windows machine

koji

Tried this branch on Windows machine and it worked as expected!
🚀

sfoster1

Thank you! Looks good!

mjhuff and others added 2 commits September 11, 2023 16:54

mjhuff requested a review from a team September 12, 2023 12:41

mjhuff requested a review from a team as a code owner September 12, 2023 12:41

mjhuff requested review from koji and removed request for a team September 12, 2023 12:41

jerader approved these changes Sep 12, 2023

View reviewed changes

sfoster1 requested changes Sep 12, 2023

View reviewed changes

requested changes

797cf4b

remove unnecessary typing

31a35be

koji approved these changes Sep 12, 2023

View reviewed changes

don't break the app if the old protocols dir doesn't exist

001df5e

mjhuff force-pushed the app-shell_support-protocol-backwards-app-compatibility branch from 4008bed to 001df5e Compare September 12, 2023 17:44

sfoster1 approved these changes Sep 12, 2023

View reviewed changes

mjhuff merged commit 1dccc06 into chore_release-7.0.0 Sep 12, 2023

mjhuff deleted the app-shell_support-protocol-backwards-app-compatibility branch September 12, 2023 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(app, app-shell): Support protocol backwards app compatibility #13518

fix(app, app-shell): Support protocol backwards app compatibility #13518

mjhuff commented Sep 12, 2023 •

edited

Loading

codecov bot commented Sep 12, 2023 •

edited

Loading

jerader left a comment

jerader Sep 12, 2023

mjhuff Sep 12, 2023

sfoster1 left a comment

sfoster1 Sep 12, 2023

sfoster1 Sep 12, 2023

mjhuff commented Sep 12, 2023

koji commented Sep 12, 2023

koji left a comment

sfoster1 left a comment

		// TODO(jh, 2023-09-11): remove OLD_PROTOCOLS_DIRECTORY_PATH after
		// OT-2 parity work is completed and move all protocols back to "protocols" directory.

fix(app, app-shell): Support protocol backwards app compatibility #13518

fix(app, app-shell): Support protocol backwards app compatibility #13518

Conversation

mjhuff commented Sep 12, 2023 • edited Loading

Overview

The Immediate Problem

The Future Problem

Test Plan

Changelog

Risk assessment

codecov bot commented Sep 12, 2023 • edited Loading

Codecov Report

jerader left a comment

Choose a reason for hiding this comment

jerader Sep 12, 2023

Choose a reason for hiding this comment

mjhuff Sep 12, 2023

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

sfoster1 Sep 12, 2023

Choose a reason for hiding this comment

sfoster1 Sep 12, 2023

Choose a reason for hiding this comment

mjhuff commented Sep 12, 2023

koji commented Sep 12, 2023

koji left a comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

mjhuff commented Sep 12, 2023 •

edited

Loading

codecov bot commented Sep 12, 2023 •

edited

Loading