oxidecomputer/dropshot

Schemars versions > v0.8.17 breaks OpenAPI Error definition

Closed this issue ยท 8 comments

TL;DR: I'm currently unsure where in the schemars code this regression occurs, but it seems that above version 0.8.17 schemars seems to break the previous functionality of making error_code in the Error definition not required. Which might be confusing for users who simply run cargo add schemars

I'm somewhat down a rabbithole, but I figured I would make a post and report back what I've found so far. Maybe it's helpful, maybe it's not. This is mostly just a suggestion that we probably want to add a note or update the readme/example suggestions so that other people don't fall down the same pit. (or maybe it's better we knock on schemars door to report the breaking change outside a major/minor version?).

How I got here

I noticed while using dropshot for my own project (I love it by the way!) that while writing the CLI I was getting json parsing errors whenever getting error responses from my API.

After digging for a bit I noticed that the openapi specification that was being generated had error_code as a required field. Which was obviously odd because the HttpError struct does not.

Long story short, after attempting to create a minimum viable reproduction from the basic.rs file in the examples I was even more confused because I could not reproduce the openapi error definition I was seeing affirmed by dropshot's tests. Every definition included error_code as a required field.

What I found

After bouncing around for a bit trying to figure out which package was in the wrong(progenitor, dropshot, schemars) I decided to check which exact version of each I was using. Due to the crate semvar rules I had schemars 0.8.17 in my cargo toml but I was by default using schemars 0.8.19.

Simply specifying version = "=0.8.17" in my cargo toml seemed to fix the problem.

I'm currently unsure where in the schemars code this regression occurs, but it seems that above version 0.8.17 schemars seems to break the previous functionality of making error_code in the openapi Error definition not required.

How to reproduce

I simply took the examples/basic.rs file and added:

    let mut output = Cursor::new(Vec::new());

    let _ = api
        .openapi("test", "1985.7")
        .description("gusty winds may exist")
        .contact_name("old mate")
        .license_name("CDDL")
        .terms_of_service("no hat, no cane? no service!")
        .write(&mut output);

    let actual = from_utf8(output.get_ref()).unwrap();
    println!("{}", actual);

You can test that it fails on specific (acceptable patch versions) by simply changing the cargo.toml dependency definition back and forth from schemars = "=0.8.17" to schemars = "=0.8.18" (Making sure to use the = modifier to force rust to use that specific version).

The output from the two will be different in a way that would break dropshot's assumptions for anyone new generating a brand new project.

The regression occurs in 0.8.18, which only has one PR: GREsau/schemars#286

ahl commented

Should be fixed not

@ahl Maybe time for a patch release of Dropshot 0.10.1 to crates.io? That way others can use dropshot with the latest schemars.

ahl commented

Good point; we'll get it out there.

ahl commented

Done