Handle `external-state` plugin errors with `ErrorController` by KVSRoyal · Pull Request #868 · player-ui/player

KVSRoyal · 2026-05-16T02:45:50Z

Now that ErrorController is available, we use that to handle errors from the external-state plugin instead of letting it fall through. See included docs updates for more details.

Change Type (required)

Indicate the type of change your pull request is:

patch
minor
major
N/A. Part of 1.0

Does your PR have any documentation updates?

Updated docs
No Update needed
Unable to update docs

codecov · 2026-05-16T03:01:27Z

Bundle Report

Changes will increase total bundle size by 382.95kB (8.0%) ⬆️⚠️, exceeding the configured threshold of 5%.

Bundle name	Size	Change
plugins/check-path/core	442.6kB	41 bytes (0.01%) ⬆️
plugins/async-node/core	504.25kB	41 bytes (0.01%) ⬆️
plugins/markdown/core	683.26kB	41 bytes (0.01%) ⬆️
plugins/asset-transform/core	11.25kB	3.34kB (42.22%) ⬆️⚠️
plugins/reference-assets/core	554.9kB	41 bytes (0.01%) ⬆️
plugins/beacon/core	424.2kB	41 bytes (0.01%) ⬆️
plugins/metrics/core	460.76kB	41 bytes (0.01%) ⬆️
core/player	1.02MB	146 bytes (0.01%) ⬆️
plugins/external-state/core	426.87kB	379.22kB (795.79%) ⬆️⚠️

Affected Assets, Files, and Routes:

view changes for bundle: plugins/async-node/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`AsyncNodePlugin.native.js`	41 bytes	441.63kB	0.01%

view changes for bundle: plugins/beacon/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`BeaconPlugin.native.js`	41 bytes	409.73kB	0.01%

view changes for bundle: plugins/asset-transform/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`index.legacy-esm.js`	3.34kB	3.34kB	100.0% ⚠️

view changes for bundle: core/player

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`Player.native.js`	41 bytes	426.34kB	0.01%
`cjs/index.cjs`	35 bytes	202.28kB	0.02%
`index.legacy-esm.js`	35 bytes	195.55kB	0.02%
`index.mjs`	35 bytes	195.55kB	0.02%

view changes for bundle: plugins/external-state/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`ExternalStatePlugin.native.js`	373.57kB	407.9kB	1088.21% ⚠️
`cjs/index.cjs`	1.92kB	7.08kB	37.3% ⚠️
`index.legacy-esm.js`	1.86kB	5.94kB	45.54% ⚠️
`index.mjs`	1.86kB	5.94kB	45.54% ⚠️

view changes for bundle: plugins/reference-assets/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`ReferenceAssetsPlugin.native.js`	41 bytes	516.92kB	0.01%

view changes for bundle: plugins/markdown/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`MarkdownPlugin.native.js`	41 bytes	658.11kB	0.01%

view changes for bundle: plugins/metrics/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`MetricsPlugin.native.js`	41 bytes	428.42kB	0.01%

view changes for bundle: plugins/check-path/core

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`CheckPathPlugin.native.js`	41 bytes	413.12kB	0.01%

codecov · 2026-05-16T03:12:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (30198e7) to head (eb34b91).

Additional details and impacted files

@@           Coverage Diff            @@
##   player-1-dot-zero   #868   +/-   ##
========================================
========================================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sugarmanz · 2026-05-18T20:19:01Z


-  test("no handler registered for external state - no transition occurs", async () => {
-    // Create a player with NO handlers registered
+  test("no handler registered for external state - calls captureError with ExternalStateError", async () => {


Can we get some tests to validate how you'd handle this error by the error controller? My understanding is there are two ways, one via content and another by tapping onError. I think these tests would be valid for each platform, at least the onError ones since external actions are closer to integrations.

sugarmanz · 2026-05-18T20:21:13Z

+                this.errorController?.captureError(
+                  ExternalStateError.missingTransitionValue(toState.value.ref),
+                );
+                return;


How should the error be handled if we there isn't an errorController? Right now it'd fail silently potentially leaving the user stuck with a blank or stale screen. It'd likely be worth logging up front so that integrators know when this happens, or maybe a check at the start to ensure we have what we need to continue, like the InProgress state check below.

Good call. Adding

sugarmanz · 2026-05-18T20:24:09Z

+  extends Error
+  implements PlayerErrorMetadata<ExternalStateErrorMetadata>
+{
+  readonly type: string = "EXTERNAL-STATE";


Can we follow the same convention for types as the primary type definition, i.e. lowercase?

JunDangIntuit · 2026-05-19T14:18:31Z

+  extends Error
+  implements PlayerErrorMetadata<ExternalStateErrorMetadata>
+{
+  readonly type: string = "externalState";


May we add this to ErrorTypes so it's the single source of truth. for example: export const ErrorTypes = {
EXTERNAL_STATE: "externalState",
}
Drop the : string annotation: readonly type = ErrorTypes.EXTERNAL_STATE. Since ErrorTypes is as const, the field infers the literal "external-state". Because string widens it and breaks narrowing — TS can't tell this error apart from others whose type is also typed as string.

Will do! Good call

Er, except I probably will keep string since that's the pattern used in other places.

brocollie08 · 2026-05-28T19:30:15Z

+    @TestTemplate
+    fun testMissingHandlerNavigatesViaContentErrorTransitions() = runBlockingTest {
+        val plugin = ExternalStatePlugin(
+            ExternalStateHandler(ref = "different-ref") { _, _, transition ->


hmmmm is having no handler expected to go into an error state? Do we not get a bunch of no-ops if there's no handler found for the ref in the registry? Could you show me where it errors out

Hi! It goes into an error state, yeah. Here is where the core code errors out.

KVSRoyal marked this pull request as ready for review May 16, 2026 03:17

KVSRoyal requested review from a team as code owners May 16, 2026 03:17

sugarmanz reviewed May 18, 2026

View reviewed changes

JunDangIntuit reviewed May 19, 2026

View reviewed changes

KVSRoyal requested review from a team as code owners May 26, 2026 16:21

KVSRoyal added 3 commits May 26, 2026 10:52

captureError for External State

d589d82

Revise per pr review

1a623fb

Add mobile tests, add externalState to ErrorTypes

3a9f697

KVSRoyal force-pushed the external-action-error branch from 3f7a804 to 3a9f697 Compare May 26, 2026 16:52

sugarmanz added 13 commits May 26, 2026 11:23

update bazel@8.4.2 and rules_android@0.7.2

a55ba55

remove outdated config

7e6248f

use PR image to test

557a3a9

use PR image to test

2780134

upgrade to android-35 in android-test job

7ff87bf

upgrade to android-35 image in android-test job

70f77c4

upgrade to android-35 image in android-test job

acc6c8d

upgrade to android-35 image in android-test job

3751ed6

upgrade to android-35 image in android-test job

912843f

pin rules_java < 9.0

1fba752

test flake fix

6bc79c5

add missing import

6ad6ed7

introduce clickClickableSpan to address API 35 changes

0f7c332

brocollie08 reviewed May 28, 2026

View reviewed changes

Use latest docker

e4c1b6d

JunDangIntuit approved these changes May 28, 2026

View reviewed changes

Update InputTest.kt

eb34b91

KVSRoyal requested a review from brocollie08 May 28, 2026 22:18

brocollie08 approved these changes Jun 1, 2026

View reviewed changes

KVSRoyal merged commit 9aa8a41 into player-1-dot-zero Jun 3, 2026
19 checks passed

KVSRoyal deleted the external-action-error branch June 3, 2026 22:55

Conversation

KVSRoyal commented May 16, 2026

Change Type (required)

Does your PR have any documentation updates?

Uh oh!

codecov Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle Report

Affected Assets, Files, and Routes:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Assets Changed:

Uh oh!

codecov Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunDangIntuit May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 16, 2026 •

edited

Loading

codecov Bot commented May 16, 2026 •

edited

Loading

JunDangIntuit May 19, 2026 •

edited

Loading