Skip to content

fix(brp): allow params and id to be undefined when deserializing#23661

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
kfc35:23643_optional_params_and_id
Apr 6, 2026
Merged

fix(brp): allow params and id to be undefined when deserializing#23661
alice-i-cecile merged 1 commit intobevyengine:mainfrom
kfc35:23643_optional_params_and_id

Conversation

@kfc35
Copy link
Copy Markdown
Contributor

@kfc35 kfc35 commented Apr 4, 2026

Objective

Solution

  • during deserialization, params and id should just flatten the nested Option instead of unwrap and assuming something is there

Testing

  • Wrote a regression test
  • Tested executing a curl while the server example without params, and it works
    curl -d'{"jsonrpc":"2.0","method":"world.list_components","id":1}' -X POST -H "Accept: applcation/json" -H "Content-Type: application/json" http://127.0.0.1:15702
  • Tested schedule.graph to ensure that it panics without params provided on its own (it does) and that params are still read correctly when provided (it does)

@kfc35 kfc35 added C-Bug An unexpected or incorrect behavior P-Regression Functionality that used to work but no longer does. Add a test for this! A-Dev-Tools Tools used to debug Bevy applications. D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 4, 2026
@kfc35 kfc35 added this to the 0.19 milestone Apr 4, 2026
@kfc35 kfc35 force-pushed the 23643_optional_params_and_id branch from 7478bd4 to 626f51e Compare April 4, 2026 18:50
@kfc35 kfc35 changed the title fix(brp): allow params and id to be undefined fix(brp): allow params and id to be undefined when deserializing Apr 4, 2026
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 6, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 6, 2026
Merged via the queue into bevyengine:main with commit 9f90ff3 Apr 6, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Dev-Tools Tools used to debug Bevy applications. C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BRP requests without params are rejected, even for paramless methods

3 participants