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

Preferred Roc Error Handling? #21

Open
penland365 opened this issue Mar 14, 2016 · 1 comment
Open

Preferred Roc Error Handling? #21

penland365 opened this issue Mar 14, 2016 · 1 comment

Comments

@penland365
Copy link
Contributor

Problem

What is the appropriate way to handle errors in Roc? There are several important factors weighing in on this decision

  1. We are not bound by JDBC style of error handling.
  2. Postgresql has explicit documentation around "error classes" that allows us to model a great many Server errors without the need to create Exceptions
  3. From point 2., there are 278 explicit types of errors Postgresql can emit, and 41 "error classes" that these 278 fall into.
  4. From point 3., these 41 Error Class codes can be further classified as "Error", "Fatal", "Panic", "Warning", "Notice", "Debug", "Info", "Log" to denote the expected action of a client.
  5. "Adhere to the style of the original" suggests that we should not stray too far from the Twitter.Future.exception style of handling.
  6. The Result type is more or less undefined at this point - currently it just holds some information returned from the DB, but this isn't firm in the slightest and I expect it to change dramatically as the code continues to reveal itself.

Currently, we are modeling any Errors that happen in the client ( for example, decoding failures ) as Failures, and any Exceptions that occur on the postgresql side as PostgresqlErrors

abstract class Failure extends Exception // Client side problems
abstract class PostgresqlError  // Server side problems

Possible Solutions

The "kill -9"

Perhaps the easiest of these, we create a simple type to model any error from a Postgresql database. Upon receipt of any error, we immediately close the connection and return a

case class PostgresqlError extends Exception
 Future.exception(new PostgresqlError())

The "all. the. errors." approach

sealed abstract class PostgresqlError // does NOT extend Exception
case class UndefinedColumn extends PostgresqlError

Under this approach, we explicitly model each Postgresql Error type. As the errors are a closed set ( i.e., there is a well defined set of errors and a well defined behavior if, for some reason, an error is returned that does not appear in this set), this is certainly a possibility. The connection would not be closed, and the Result type would have a way to model a possible error ( perhaps as a disjunction? ).

However, the enormous number of error types makes this challenging and error prone for any user.

The "adhere to JDBC style" approach

In this scenario, we would map Postgreql Errors to their corresponding JDBC Exceptions, and then simply bail out in the client code:

// some decoding has occurred above
val error = InvalidColumnReference
Future.exception(new java.sql.SQLException(error))

The "let's just cut the baby in half" approach

In this scenario, we would create Error Types based off the severity level. Errors that do not affect the connection state would simply be errors, while errors that do affect the connection state would also be Exceptions, i.e.

sealed abstract class PostgresqlError
case class Error extends PostgresqlError // example, "no table named FOO exists"
case class Fatal extends PostgresqlError with Exception // example, admin shutdown initiated
case class Panic extends PostgresqlError with Exception // example, OOM
case class Debug extends PostgresqlError

Non-exception types would be returned as the Result type (still not sure how though), while any connection closing errors would close the connection, and then bail out via

Future.exception(new Fatal())

Thoughts on this? I'm kind of torn between the "shove everything into a Future.exception" and "model all error types".

@jcoveney
Copy link

I like the general idea of the "cut the baby in half" approach, except I guess I personally don't see the Exception as a sign of "fatality"...to me, exceptions are "do you want information on the place where this happened?" and nothing more, because I dislike stack busting (which is their other use in the JVM).

I such, I like the idea that there is a Postgressql error class, and then types for the expected actions, and then the rest can be untyped... so you would still tag it with the fine grained information (specific error code, error information, blah blah, and have helpers to manage that as necessary). I don't think it's necessary to model every error, and I also don't think it's necessary to get super fine-grained about "is this an exception or not."

Others may disagree!

penland365 referenced this issue Mar 22, 2016
Following an initial discussion https://github.com/penland365/roc/issues/21
on preferred Error Handling, the early implementation of PostgresqlError
was replaced by PostgresqlMessage. Following the example set in
Postgresql source code, all PostgresqlMessage were furthur classified
into one of Success / Warning / Error / Unknown. A new failure type was
introduced, PostgresqlServerFailure, to contain any ErrorMessage sent
by a Postgresql Server.

This implementation allows us to much more easily handle
NoticeResponses, and allows us to have a more nuanced process ( yet to
be implemented ) around connection handling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants