[BUG] D1 binding doesn't support Null values

[v-rogg]

Is there an existing issue for this?

• [X] I have searched the existing issues

What version of workers-rs are you using?

0.4.2

What version of wrangler are you using?

3.93.0

Describe the bug

I have a DB with a nullable column. The following code works if the value is not NULL. As soon as the value is NULL I get this error: Error: Error: invalid type: JsValue(null), expected String - Cause: Error: invalid type: JsValue(null), expected String

#[derive(Deserialize, Serialize)]
struct ExampleObject {
    id: String,
    nullable: Option<String>,
}

#[event(fetch)]
async fn fetch(req: Request, env: Env, _ctx: Context) -> Result<Response> {
    Router::new()
        .get_async("/", |req, ctx| async move {
            let db = ctx.env.d1("DB")?;

            let query = db.prepare("SELECT id FROM example");
            let result = query.first::<ExampleObject>(None).await?;
            match result {
                Some(res) => Response::from_json(&res),
                None => Response::error("", 404),
            }
        })
        .run(req, env)
        .await
}

With a similar setup, trying to insert a NULL value I get this error: Error: Error: D1_TYPE_ERROR: Type 'object' not supported for value '[object ExecutionContext]' - Cause: Error: D1_TYPE_ERROR: Type 'object' not supported for value '[object ExecutionContext]'

Currently a workout could be to use .raw_js_values() instead of .first(), .all(), .raw()

[spigaz]

I have spent a few hours on the problem of persisting none values as nulls.

It appears as a problem during serialisation of the arguments, but in practice the macro seems to be working correctly, converting None to wasm_bindgen::JsValue::NULL.

But the wasm_bindgen::JsValue::NULL instead of returning a valid null value is returning the environment variable with all the secrets and so on, yes this object:

D1_TYPE_ERROR: Type 'object' not supported for value '[object ExecutionContext]

just try

console_error!("Null value: {:?}", wasm_bindgen::JsValue::NULL);

I already added some test cases, updated dependencies, but I have no idea of where to look.

I suspect that when the Env is imported to the Rust Land from the JS Land it overwrites the memory used by JsValue, I have to investigate that...

Any hints are appreciated... :)

[kflansburg]

But the wasm_bindgen::JsValue::NULL instead of returning a valid null value is returning the environment variable with all the secrets and so on, yes this object:

Wow, that is very spooky!

Is this serialization or deserialization? Seems to be deserialization in the original code example. Perhaps it is impacting both?

Might be worth testing different versions of wasm-bindgen to see if there was a regression.

[spigaz]

I detected the issue while testing serialisation for a none value, because the None was converted to that object instead of to null.

#[worker::send]
pub async fn serialize_none(_req: Request, _env: Env, _data: SomeSharedData) -> Result<Response> {
    console_error_panic_hook::set_once();
    let serializer = serde_wasm_bindgen::Serializer::new().serialize_missing_as_null(true);
    
    console_error!("Null value: {:?}", wasm_bindgen::JsValue::NULL);

    let none: Option<String> = None;
    let js_none = ::serde::ser::Serialize::serialize(&none, &serializer).unwrap();
    assert!(js_none.is_null());

    Response::ok("ok")
}

Seems to be deserialization in the original code example.

Correct, but I haven't tested deserialisation independently yet. I have a serialize then deserialize example, that got stuck here, but I can add that also.

I was more concerned in trying to understand what was wrong with the serialisation, till I did this console_error.

Might be worth testing different versions of wasm-bindgen to see if there was a regression.

Well, it might be, I have to test that. Thanks!

PS: The main problem in these kind of situations is that this wasn't covered by tests previously.

On another perspective, some other tests are broken also, R2 and Cache I believe, I have to give them a look.

[spigaz]

I created a PR, but it only fixes the first when it should return None. But the tests might be relevant to anyone that wants to give it a try.

As I suspected the problem is focused on JsValue::NULL even on deserialisation, I believe that it checks the JsValue(null) it finds against it, it doesn't match, so it moves on to consider that is a Some value, therefor the error that it gives.

I haven't checked different versions yet, probably I'm going to investigate why it doesn't happen in my app first, JsValue::NULL is fine, although I don't use D1 and use web_sys.

[spigaz]

@v-rogg I added a PR just to fix the first behaviour and added some test cases to avoid future regressions, but AFAIK everything should be working just fine now.

Next time you try to run wrangler if your build in wrangler.toml is like this

[build]
command = "cargo install -q worker-build && worker-build --release"

The option issues should be gone, except for the first issue that will have to wait till it gets merged.

[v-rogg]

Thanks for looking into it.

Using the custom build command resolved the issue for me as well.

Thanks to you, I could finish my worker 👍

[guybedford]

We landed the test cases from this report in the latest release, so I believe this should no longer be an issue here.