Skip to content

Conversation

bitfl0wer
Copy link
Contributor

@bitfl0wer bitfl0wer commented Jul 15, 2025

I have encountered a two separate program-crashing errors, where, when calling .schema(&spec) on a MediaType or .request_body(&spec) on a RequestBody, the program will panic, if the inner Option<ObjectOrReference<_>>> is None. This should not happen, especially because the methods in question already return a Result<T, E> type, which allow for error handling at the call site.

This PR simply substitutes the unwrap() calls with an ok_or()-call, which yields a newly defined RefError variant "NoRef", if the inner Option<ObjectOrReference<_>>> is None.

Edit: Since I have also encountered the original error when trying to resolve a potentially None RequestBody reference, I updated the PR to fix that .unwrap() call as well.

@bitfl0wer
Copy link
Contributor Author

@robjtede awawawawawawawawawawawawawawaw

@robjtede
Copy link
Member

My development laptop was stolen last week so I'm unable to check this out right now.

@bitfl0wer
Copy link
Contributor Author

Oh. I am terribly sorry to hear that D: In that case, no rush! :)

@bitfl0wer bitfl0wer force-pushed the flori/no-unwrap-schema-resolve branch from 6483ea7 to 4edfea4 Compare July 21, 2025 14:35
@bitfl0wer bitfl0wer changed the title Do not .unwrap() on potentially None schema Do not .unwrap() on potentially None references Jul 21, 2025
feat: add new `Error` variant `NoReference`
@bitfl0wer bitfl0wer force-pushed the flori/no-unwrap-schema-resolve branch from 4edfea4 to e2cf38b Compare July 22, 2025 15:09
@robjtede
Copy link
Member

robjtede commented Aug 26, 2025

What do you think about instead, either:

  • changing these methods to return -> Result<Option<ObjectSchema>, Error>.
  • drop the Option wrapper from MediaType.schema field.
    • I guess we can't do that for Schema.request_body so returning Result-Option definitely makes sense there.

@robjtede
Copy link
Member

Looking at the option-ality of these fields, I think this is the correct solution for the library: #239

@robjtede robjtede closed this Aug 26, 2025
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.

2 participants