Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tokio-Postgres] Error handling #790

Open
JojiiOfficial opened this issue Jun 16, 2021 · 13 comments
Open

[Tokio-Postgres] Error handling #790

JojiiOfficial opened this issue Jun 16, 2021 · 13 comments

Comments

@JojiiOfficial
Copy link

How to check against a certain error::Kind given a result eg coming from query_one(...)?
The kind enum is private but I need to check somehow against Kind::RowCount in order to check whether its a 'real' error or if simply no entry was found.

@sfackler
Copy link
Owner

If you want to programmatically interact with the number of rows returned, I would recommend just using client.query instead.

@JojiiOfficial
Copy link
Author

So I have to unnecessarily allocate a vector, load all entries from the database, even though I always want just one single item? Why is the error type almost entirely closed?

@sfackler
Copy link
Owner

You are going to be loading all entries from the database regardless of if you consume them or not in the client. If you have carefully benchmarked your application and found that the time spent allocating a one element vector for this process is significant, then you can take the approach in the implementation of query_one: https://github.com/sfackler/rust-postgres/blob/master/tokio-postgres/src/client.rs.

The intended use case for query_one is if any number of rows other than exactly 1 is a programming error.

@JojiiOfficial
Copy link
Author

Wouldn't be an own implementation of query_one() a much bigger effort than simply making the error type and Kind public and providing some sort of get_kind() function? It needs to be maintained and its unnecessarily duplicated code. What is the reason for the error type being private?

It is not necessarily about the performance impact of allocating a new vector but I don't see any reason to do so in this chase since simply providing a more public error type would fix this issue and probably more.

Would you agree with a PR which improves this error handling by making the error type and the Kind enum more accessible?

@sfackler
Copy link
Owner

Can you explain more concretely what you want to do when you encounter a row count error? The kind is made opaque to provide more flexibility in the future.

Repository owner deleted a comment from Badreah87 Jun 18, 2021
@l-7-l
Copy link

l-7-l commented Jul 25, 2021

Can you explain more concretely what you want to do when you encounter a row count error? The kind is made opaque to provide more flexibility in the future.

I have some immature opinions, database errors may be divided into two types:

  1. Infrastructure errors: such as timeout, connection closed
  2. Operation error: not found and some other similar

In my project, the format of data model and data table is inconsistent, i need to manually convert it and map tokio-postgre Error to Custom Error in order to return the corresponding service status code instead of all returning 500 or 404 or similar

   // model
   struct Base<ID> {
     id: ID,
     created_at: DateTime<Utc>
   }
   type ModelID = SomeStringID;
    struct Model {
        base: Base<ModelId>,
        name: String
    }
  -- table
  id: text,
   name: text
  created_at: timestamptz

so I think it is necessary to make ErrorKind public or make query_one result optional?

@sfackler
Copy link
Owner

or make query_one result optional?

https://docs.rs/tokio-postgres/0.7.2/tokio_postgres/struct.Client.html#method.query_opt

@johnmave126
Copy link

Encountered similar issue.
I am just wondering whether there is a easy way to differentiate between "too many rows returned" and "database connection is closed"? It seems that both these 2 errors only have kind but not cause.

@sfackler
Copy link
Owner

Count the rows yourself.

@stanislav-tkach
Copy link

I also find this inconvenient. In my case I'm dealing with a library that returns a custom error with tokio_postgres::Error inside one of its variants. Sure it would be better to return Result<Option<T>, tokio_postgres::Error> from the library itself, but I will be able to work this around in my code if the error kind in the tokio_postgres is public.

@beowulf1416
Copy link

i'm trying to handle this error by comparing the Error result in an if statement which doesn't work.
if e == Error::row_count()
doesn't work because the row_count function is private. can you turn make it public or turn it into an enum? is there another way to do this?

@ehrktia
Copy link

ehrktia commented Apr 22, 2024

am having a similar issue as above, using query_one to check if a record exists in the table.
if not it returns the tokio_postgres::Errror. This error covers both infra and operation errors.Would be helpful to
have a kind() operation on an error as the public interface.
As a work around am trying to use Row.is_empty() from the result to determine the error kind.

@BaptisteRoseau
Copy link

BaptisteRoseau commented May 6, 2024

I believe it would be better to enable users to match on the enum kind to give them more control for error conversion.

This can simply be done by making the Kind enum public and adding a getter en the error struct.

For example, as a user library I want to be able to convert the error for specific cases:

#[derive(thiserror::Error)]
pub(crate) enum MyDatabaseError {
    #[error("'{0}' does not exist")]
    NotFound(String),
    #[error("Cannot add this resource as it already exists")]
    AlreadyExists,
    #[error("Unable to authenticate database")]
    Authentication,
    #[error(transparent)]
    PostgresError(tokio_postgres::Error),
}

impl From<tokio_postgres::Error> for MyDatabaseError {
    fn from(err: tokio_postgres::Error) -> Self {
        match err.kind() {
            Kind::RowCount => DatabaseError::AlreadyExists,
            Kind::Authentication => DatabaseError::Authentication,
            Kind::RowCount => DatabaseError::NotFound("Maybe extract more info from 'err' here to specify what was not found".to_string()),
            _ => DatabaseError::PostgresError(err),
        }
    }
}

It is not convenient to manually count the rows when we know only one is expected. Rust offers such a rich way to deal with errors, we definitely should give more read-only control to the user.

Edit: I made a pull request with the modification, see #1138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants