JSON serialization error panics with PostgreSQL

[FSMaxB]

Bug Description

When using sqlx::Json in sqlx::query!, if serializing the value fails, it panics with

'failed to serialize to JSON for encoding on transmission to the database: Error("the enum variant Variant::Unknown cannot be serialized", line: 0, column: 0)'

The line where this happens is:

sqlx/sqlx-postgres/src/types/json.rs

Lines 74 to 75 in d0fbe7f

	serde_json::to_writer(&mut **buf, &self.0)
	.expect("failed to serialize to JSON for encoding on transmission to the database");

Not sure how to fix this without a breaking change because Encode::encode_by_ref returns IsNull and not a Result of some kind. I would still argue that this shouldn't panic though.

Minimal Reproduction

I created a small reproducer here: https://github.com/FSMaxB/sqlx-serialization-panic-reproducer

This is the same reproducer but without the full project setup.

use serde::{Deserialize, Serialize};
use sqlx::{Connection, PgConnection};
use sqlx::types::Json;

#[tokio::main(flavor = "current_thread")]
async fn main() -> anyhow::Result<()> {
    let mut connection = sqlx::PgConnection::connect("postgresql://postgres:postgres@localhost:14538").await?;

    // this goes through successfully
    insert_variant(&mut connection, Variant::A).await?;

    // this panics
    insert_variant(&mut connection, Variant::Unknown).await?;
    // And yes, writing code like this should be considered a bug, because this
    // can obviously never work with a non-serializable variant, but I would
    // strongly argue that this should not panic (especially since this is an easy
    // mistake to make)

    Ok(())
}

async fn insert_variant(connection: &mut PgConnection, variant: Variant) -> sqlx::Result<()> {
    sqlx::query!(
        "INSERT INTO variants (variant) VALUES ($1)",
        Json(variant) as _,
    ).execute(connection).await?;

    Ok(())
}

#[derive(Clone, Copy, Deserialize, Serialize)]
enum Variant {
    A,
    B,
    C,
    #[serde(other, skip_serializing)]
    Unknown,
}

I chose #[serde(skip_serializing)] to provoke the serialization error in this example, but it could also have been any other serialization error.

Info

• SQLx version: 0.7.1
• SQLx features enabled:
  • migrate
  • json
  • macros
  • postgres
  • runtime-tokio-rustls
• Database server and version: PostgreSQL 15.4
• Operating system: Linux 6.4.12-200.fc38.x86_64 x86_64
• rustc --version: rustc 1.72.0 (5680fa18f 2023-08-23)

[FSMaxB]

@abonander Is this something you would consider for the next breaking release?

I can try taking this on. So far I tried making encode and encode_ref return Result<IsNull, BoxDynError>, this bubbles up all the way and even ends up in bind(..). Maybe this can be solved by storing the error internally and only surfacing it on .execute(..) etc. instead of on every .bind(...).

[abonander]

Yep, we've started putting together 0.8.0 so now is the time to do it! The infallibility requirement has bitten us in a few other places as well (#3053).

I like the idea of storing the error internally until .execute() is called to avoid requiring every usage everywhere to be refactored; I'd add .try_bind() to eagerly get the error where the user wants it.

[FSMaxB]

I've finally gotten around to finishing up my PR.