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

Changes to serde-field data cause crash when loading blueprint #3127

Closed
jleibs opened this issue Aug 28, 2023 · 0 comments · Fixed by #3130
Closed

Changes to serde-field data cause crash when loading blueprint #3127

jleibs opened this issue Aug 28, 2023 · 0 comments · Fixed by #3130
Labels
🟦 blueprint The data that defines our UI 🪳 bug Something isn't working
Milestone

Comments

@jleibs
Copy link
Member

jleibs commented Aug 28, 2023

As we modify blueprint data we can end up with a situation where moving between releases leaves blueprints in a bad state and subsequently crashes on load:

thread 'ThreadId(1)' panicked at 'called `Option::unwrap()` on a `None` value', arrow2_convert-0.5.0/src/deserialize.rs:33

   0: backtrace::backtrace::trace
   1: backtrace::capture::Backtrace::new

Andreas fixed this while developing #3078 but we should back-port just the fix portion into 0.8.2 so that when we break things in 0.9.0 and users go back to 0.8.2 they don't get a crash.

This basically needs these three commits:

@jleibs jleibs added 🪳 bug Something isn't working 🟦 blueprint The data that defines our UI labels Aug 28, 2023
@jleibs jleibs added this to the 0.8.2 milestone Aug 28, 2023
jleibs added a commit that referenced this issue Aug 28, 2023
### What
arrow2-convert forces us into needing to provide data even if serde fail
to deserialize the field. Fall back to default in this case, but specify
invalid.

Additionally, this checks to see if a blueprint is totally invalid and
if so resets it to the default state. Because this can reset the
blueprint immediately upon loading, it was important that this properly
set auto-space-views. Needed to pull in a slightly different version of
#3077 that still works with the
now-required usage of Default.

Resolves: #3127

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3129) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3129)
- [Docs
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

---------

Co-authored-by: Andreas Reich <[email protected]>
jleibs added a commit that referenced this issue Aug 31, 2023
### What
arrow2-convert forces us into needing to provide data even if serde fail
to deserialize the field. Fall back to default in this case, but specify
invalid.

Additionally, this checks to see if a blueprint is totally invalid and
if so resets it to the default state. Because this can reset the
blueprint immediately upon loading, it was important that this properly
set auto-space-views. Needed to pull in a slightly different version of
#3077 that still works with the
now-required usage of Default.

Resolves: #3127

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3129) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3129)
- [Docs
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

---------

Co-authored-by: Andreas Reich <[email protected]>
jleibs added a commit that referenced this issue Aug 31, 2023
### What
arrow2-convert forces us into needing to provide data even if serde fail
to deserialize the field. Fall back to default in this case, but specify
invalid.

Additionally, this checks to see if a blueprint is totally invalid and
if so resets it to the default state. Because this can reset the
blueprint immediately upon loading, it was important that this properly
set auto-space-views. Needed to pull in a slightly different version of
#3077 that still works with the
now-required usage of Default.

Resolves: #3127

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3129) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3129)
- [Docs
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

---------

Co-authored-by: Andreas Reich <[email protected]>
jleibs added a commit that referenced this issue Aug 31, 2023
### What
arrow2-convert forces us into needing to provide data even if serde fail
to deserialize the field. Fall back to default in this case, but specify
invalid.

Additionally, this checks to see if a blueprint is totally invalid and
if so resets it to the default state. Because this can reset the
blueprint immediately upon loading, it was important that this properly
set auto-space-views. Needed to pull in a slightly different version of
#3077 that still works with the
now-required usage of Default.

Resolves: #3127

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3129) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3129)
- [Docs
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8831ec03d85499333648bb15442a39760a2ad41d/examples)
<!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW--><!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

---------

Co-authored-by: Andreas Reich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI 🪳 bug Something isn't working
Projects
None yet
1 participant