okapi icon indicating copy to clipboard operation
okapi copied to clipboard

Okapi obfuscates compiler errors

Open ThouCheese opened this issue 6 years ago • 15 comments

Thank you for this project! I've been struggling with documenting an API I build, and keeping that documentation up to date. I'm trying to add okapi to my project to have the documentation be created automatically. Unfortunately, I'm running into some issues:

As it stands, Rocket is able to provide neat Rust error messages. Consider the following example:

#![feature(decl_macro, proc_macro_hygiene)]

use rocket::{get, routes};
use rocket_contrib::json::Json;

#[derive(serde::Serialize)]
struct Response {
    reply: String,
}

#[get("/")]
fn my_controller() -> Json<Response> {
    Json(Response {
        reply: 0,
    })
}


fn main() {
    rocket::ignite()
        .mount("/", routes![my_controller])
        .launch();
}

This fails with a compile error:

[me@mycomputer tmp]$ cargo check
    Checking tmp v0.1.0 (/home/me/code/rust/tmp)
error[E0308]: mismatched types
  --> src/main.rs:17:16
   |
17 |         reply: 0,
   |                ^
   |                |
   |                expected struct `std::string::String`, found integer
   |                help: try using a conversion method: `0.to_string()`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `tmp`.

However, when the code is changed to use Okapi:

#![feature(decl_macro, proc_macro_hygiene)]

use rocket::get;
use rocket_contrib::json::Json;
use rocket_okapi::{openapi, routes_with_openapi};
use schemars::JsonSchema;

#[derive(serde::Serialize, JsonSchema)]
struct Response {
    reply: String,
}

#[openapi]
#[get("/")]
fn my_controller() -> Json<Response> {
    Json(Response {
        reply: 0,
    })
}


fn main() {
    rocket::ignite()
        .mount("/", routes_with_openapi![my_controller])
        .launch();
}

The error message changes:

[me@mycomputer tmp]$ cargo check
    Checking tmp v0.1.0 (/home/me/code/rust/tmp)
error[E0308]: mismatched types

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `tmp`.

The useful information about what caused the compilation error is gone, making it really hard to debug. Is there any way to get it back?

ThouCheese avatar Jan 28 '20 11:01 ThouCheese

I did some digging and it seems that the problem could be resolved using quote_spanned!. I don't understand the proc_macro API well enough to make the change myself however.

ThouCheese avatar Jan 28 '20 16:01 ThouCheese

tl;dr: a fix will be out soon

So this was a weird one - I think the root cause is rust-lang/rust#43081

You can see the same problem with the following repro, where both no_op1 and no_op2 should both pass through tokens unmodified (although no_op2 does it by parsing them in then quoting them back out):

///// MACRO CRATE: /////
#[macro_use]
extern crate quote;
extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn no_op1(_: TokenStream, input: TokenStream) -> TokenStream {
    input
}

#[proc_macro_attribute]
pub fn no_op2(_: TokenStream, input: TokenStream) -> TokenStream {
    let parsed_input = syn::parse::<syn::Item>(input).unwrap();
    quote!(#parsed_input).into()
}

///// APP CRATE: /////
use codegen::{no_op1, no_op2};

#[no_op1]
#[no_op1]
fn one_one() {
    let one_one: i32 = "nan";
}

#[no_op1]
#[no_op2]
fn one_two() {
    let one_two: i32 = "nan";
}

#[no_op2]
#[no_op1]
fn two_one() {
    let two_one: i32 = "nan";
}

#[no_op2]
#[no_op2]
fn two_two() {
    let two_two: i32 = "nan";
}

The output of cargo check is then:

error[E0308]: mismatched types
  |
  = note: expected type `i32`
             found type `&'static str`

error[E0308]: mismatched types
 --> app\src\main.rs:6:24
  |
6 |     let one_one: i32 = "nan";
  |                        ^^^^^ expected i32, found reference
  |
  = note: expected type `i32`
             found type `&'static str`

error[E0308]: mismatched types
  --> app\src\main.rs:18:24
   |
18 |     let two_one: i32 = "nan";
   |                        ^^^^^ expected i32, found reference
   |
   = note: expected type `i32`
              found type `&'static str`

error[E0308]: mismatched types
  --> app\src\main.rs:24:24
   |
24 |     let two_two: i32 = "nan";
   |                        ^^^^^ expected i32, found reference
   |
   = note: expected type `i32`
              found type `&'static str`

Note that one_two loses its span information, obfuscating the error. no_op1/no_op2 interact exactly the same way the openapi/get attributes do. The good news is that this also gives us a way to work-around it: by making the openapi attribute behave more like no_op2 (by passing the tokens through syn/quote), span information will be retained.

I'll try to get the fix out imminently

GREsau avatar Feb 17 '20 18:02 GREsau

This should be fixed in rocket_okapi v0.3.2, which is now published on crates.io - could you check that it works for you?

GREsau avatar Feb 17 '20 19:02 GREsau

This does indeed fix the error messages! Thank you for this fix.

ThouCheese avatar Feb 19 '20 12:02 ThouCheese

There are still some issues and I cannot pinpoint the source. Here is example of code with obfuscated error message:

#[openapi]
#[get("/")]
pub fn test(
) -> Option<Option<()>> {
    asdf
}

If you remove the second Option<> error reporting will work as expected.

johnlepikhin avatar Feb 19 '20 17:02 johnlepikhin

You are right, I just encountered this issue myself :smile:. It seems that any generic type nested with another generic does not get handled well. Reopening the issue.

ThouCheese avatar Feb 19 '20 17:02 ThouCheese

I'm not sure, but I think the cause of this may be the span bug mentioned in https://github.com/rust-lang/rust/pull/48258. And just like it says in this comment on the related issue, our issue goes away when wrapping the inner generic type in parentheses e.g.

#[openapi]
#[get("/")]
pub fn test() -> Option<(Option<()>)> {
    asdf
}

And that helped me find a work-around for this too - if the #[openapi] proc macro wraps the inner generic type(s) in parentheses, then the needed span information is preserved and compiler errors are reported correctly. This feels like a horrible hack, but it seems to do the job...

I'll publish the new fix to crates.io tonight

GREsau avatar Feb 20 '20 21:02 GREsau

Done - https://crates.io/crates/rocket_okapi 0.3.3 is available, hopefully this will fix everything!

GREsau avatar Feb 20 '20 21:02 GREsau

It now indeed works for (deeply) nested generic types. Thank you for you update!

ThouCheese avatar Feb 20 '20 22:02 ThouCheese

In the latest installment of this issue, I have found another instance of #[openapi] not producing the right error messages. It happens when I specify in the #[post] macro that the payload should be parsed, but then forget to add the argument to the function. For example

#[post("/", data = "<body>")]
fn myroute() -> String {
    "hi".to_string()
}

This produces the following error:

error: unused dynamic parameter
  --> src/main.rs:7:21
   |
7  | #[post("/", data = "<body>")]
   |                     ^^^^^^
   |
note: expected argument named `body` here
  --> src/main.rs:8:1
   |
8  | / fn myroute() -> String {
9  | |     "hi".to_string()
10 | | }
   | |_^

If I add in #[openapi] like so

#[openapi]
#[post("/", data = "<body>")]
fn myroute() -> String {
    "hi".to_string()
}

I get the following error message:

error: macros that expand to items must be delimited with braces or followed by a semicolon
 --> src/main.rs:6:1
  |
6 | #[openapi]
  | ^^^^^^^^^^
  |
help: change the delimiters to curly braces
  |
6 | {[openapi}
  | ^        ^
help: add a semicolon
  |
6 | #[openapi];
  |           ^

error: unused dynamic parameter
  --> src/main.rs:7:21
   |
7  | #[post("/", data = "<body>")]
   |                     ^^^^^^
   |
note: expected argument named `body` here
  --> src/main.rs:8:1
   |
8  | / fn myroute() -> String {
9  | |     "hi".to_string()
10 | | }
   | |_^

error: Could not find argument body matching data param.
 --> src/main.rs:6:1
  |
6 | #[openapi]
  | ^^^^^^^^^^

ThouCheese avatar Mar 03 '20 09:03 ThouCheese

Additionally, it is still possible to create unspanned error message using some functional constructs on Results and Options. Interestingly, the following code produces normal error messages:

#![feature(decl_macro, proc_macro_hygiene)]

use rocket_okapi::openapi;
use rocket::post;

#[openapi]
#[post("/sign")]
fn sign() {
    Some(1).and_then(|i| 1);
}

fn main() { }

The error message here is

error[E0308]: mismatched types
 --> src/main.rs:9:26
  |
9 |     Some(1).and_then(|i| 1);
  |                          ^
  |                          |
  |                          expected enum `std::option::Option`, found integer
  |                          help: try using a variant of the expected enum: `Some(1)`
  |
  = note: expected enum `std::option::Option<_>`
             found type `{integer}`

error: aborting due to previous error

However, if I trigger a slightly different error message like so:

#![feature(decl_macro, proc_macro_hygiene)]

use rocket_okapi::openapi;
use rocket::post;

#[openapi]
#[post("/sign")]
fn sign() {
    Some(1).and_then(|| Some(1));
}

fn main() { }

Then the error message is placed at the beginning of the file:

error[E0593]: closure is expected to take 1 argument, but it takes 0 arguments
  |
help: consider changing the closure to take and ignore the expected argument
  |
1 | |_|#![feature(decl_macro, proc_macro_hygiene)]
  | ^^^

error: aborting due to previous error

I sorry for spamming this issue without providing any help on fixing the issue, maybe if I have time somewhere by the end of this month I'll look into the proc macro api and the codebase of rocket_okapi.

ThouCheese avatar Mar 12 '20 16:03 ThouCheese

@ThouCheese I removed the above workarounds in v0.5.0, and I can no longer reproduce any of these cases. I assume this is due to internal rustc improvements, but I can't be sure.

Do you still encounter any span problems? I'm using rustc 1.45.0-nightly (7ebd87a7a 2020-05-08)

GREsau avatar May 24 '20 19:05 GREsau

Hi @GREsau,

They all generate sensible errors except for one! The one that remains is that if I do this:

#[openapi]
#[get("/", data = "<body>")]
fn myroute() -> String {
    "hi".to_string()
}

I get the following error (in addition to the warning that Rocket generates):

error: Could not find argument body matching data param.
  --> src/main.rs:13:1
   |
13 | #[openapi]
   | ^^^^^^^^^^
   |
   = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

ThouCheese avatar May 24 '20 23:05 ThouCheese

You get that error because there should be an argument body in the handler/function. See: https://rocket.rs/v0.4/guide/requests/#body-data

To indicate that a handler expects body data, annotate it with data = "<param>", where param is an argument in the handler. The argument's type must implement the FromData trait.

So it should look something like this:

#[openapi]
#[get("/", data = "<body>")]
fn myroute(body: SomeType) -> String {
    "hi".to_string()
}

ralpha avatar May 25 '20 01:05 ralpha

Yes I understand that the code is not correct, but since rocket doesn't refuse to compile I think that neither should okapi.

ThouCheese avatar May 25 '20 09:05 ThouCheese