Unhandled panics when detecting Postgres table schemas

[Ameobea]

Description

We're using sea-schema to detect the schema of Postgres tables. We've noticed that in some cases, panics occur from within sea-schema and there are no ways for us to handle them.

One of the panics that we've noticed seems to result from network blips between the service and the DB:

Ssl(ErrorStack([Error { code: 503841036, library: "DECODER routines", function: "OSSL_DECODER_from_bio", reason: "unsupported", file: "../crypto/encode_decode/decoder_lib.c", line: 101, data: "No supported data to decode. Input type: PEM" }, Error { code: 503841036, library: "DECODER routines", function: "OSSL_DECODER_from_bio", reason: "unsupported", file: "../crypto/encode_decode/decoder_lib.c", line: 101, data: "No supported data to decode. Input type: PEM" }])))

It seems that these errors are all coming from sea_schema::postgres::discovery::executor::real::Executor::fetch_all. Looking at the code, it's clear that no error handling is attempted. .unwrap().s are present in many places and errors aren't exposed to the user for handling.

This makes it difficult to use sea-query for production applications. Ideally, these query-level errors would returned back the user and the schema detection operations would all return Results.

Steps to Reproduce

Run a ton of schema detections, possibly with an unstable network.

Expected Behavior

DB errors encountered while detecting Postgres table schemas are bubbled up to the user

Actual Behavior

Query errors result in panics within the sea-schema library

Reproduces How Often

Rarely since it seems to depend on network issues or some other spurious condition

Versions

Using sea-schema v0.10.3

[tyt2y3]

SeaSchema usually only runs on developers' machine or on application startup. I guess you have a different use case here? Can you share something about it?

We have had some spring cleaning in SeaORM to remove and document panics, but this effort have yet to extend to SeaSchema.

I think it's better if we simply remove the await_all and do everything sequentially.

We will likely have to define some concrete error types. A PR will be welcomed.

[Ameobea]

We're using sea-schema to perform schema detection on unknown tables. We want to know about the columns, data types, foreign keys, and primary keys which we are able to do pretty well using this library.

I understand that this is probably a different use case than running as build scripts for getting type information for ORMs or similar. However, I do believe that most of the issues could be solved by changing some types to be promises and simply bubbling errors through.

The one downside of this would be a breaking change to sea-schema's public API. If this is acceptable, I can look into working on a PR to implement that.

[tyt2y3]

A PR is definitely welcomed!

I am okay with introducing some breaking changes to the API, e.g. wrapping all async fn's return value with a Result<T, SeaSchemaErr>.

[Ameobea]

I've put up a PR that implements that change: https://github.com/SeaQL/sea-schema/pull/109

I opted to return Result<T, sqlx::Error> to make things as simple as possible since that's really the only error type that comes up during schema detection.

Lmk what you think - happy to make changes if needed