Skip to content

fix: Serialize plan results to JSON compatible types earlier#1464

Open
tpoliaw wants to merge 1 commit intomainfrom
tuple-serialise-check
Open

fix: Serialize plan results to JSON compatible types earlier#1464
tpoliaw wants to merge 1 commit intomainfrom
tuple-serialise-check

Conversation

@tpoliaw
Copy link
Copy Markdown
Contributor

@tpoliaw tpoliaw commented Mar 31, 2026

When a plan returns a value that is serializable but contains a nested
value that is not (eg a list of an unserializable type), the check for
whether it could be serialized was too lenient and the unserializable
type would be stored only to fail later when it was sent to the message
bus.

Converting the result to a json compatible type (using mode='json')
earlier allows any issues with nested fields to be caught and handled
appropriately.

Fixes #1447

When a plan returns a value that is serializable but contains a nested
value that is not (eg a list of an unserializable type), the check for
whether it could be serialized was too lenient and the unserializable
type would be stored only to fail later when it was sent to the message
bus.

Converting the result to a json compatible type (using mode='json')
earlier allows any issues with nested fields to be caught and handled
appropriately.
@tpoliaw tpoliaw requested a review from a team as a code owner March 31, 2026 13:14
@tpoliaw tpoliaw changed the title Serialize plan results to JSON compatible types earlier fix: Serialize plan results to JSON compatible types earlier Mar 31, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.20%. Comparing base (57ed429) to head (13f6c37).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1464   +/-   ##
=======================================
  Coverage   95.20%   95.20%           
=======================================
  Files          43       43           
  Lines        3128     3129    +1     
=======================================
+ Hits         2978     2979    +1     
  Misses        150      150           

☔ 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.

@oliwenmandiamond
Copy link
Copy Markdown
Contributor

Can we please add a test with a plan MsgGenerator[tuple[AsyncStatus]] to make sure BlueAPI works with it?

Copy link
Copy Markdown
Contributor

@abbiemery abbiemery left a comment

Choose a reason for hiding this comment

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

Looks like it works, please can you add a test.

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.

When running a plan with blueapi, it completes and then forever blocks

3 participants