MySQL++

How to handle transaction errors when connection is set to no exception?
Login

How to handle transaction errors when connection is set to no exception?

(1) By bruno on 2020-02-27 04:53:21 [link] [source]

Hi,

Looking at the code, it doesn't seem that the Transaction object will handle errors if the 'ne' bit is set in the connection. Is this right? How can I handle errors in the transaction other than enabling exceptions? Is checking for the last error number good enough? What if the connection is set to auto-reconnect?

Thanks in advance,

Bruno

(2) By Warren Young (tangent) on 2020-02-27 20:57:12 [link] [source] in reply to 1

The Transaction object exists because exceptions may be happening and you want your transactions to be safely rolled back in RAII fashion when that happens. In the absence of exceptions, I'm not sure what sort of value you're expecting it to provide.

If you use Transaction without MySQL++ exceptions enabled, then of course you must do your own error checking through other objects. (Most likely the Connection or Result objects.)

Here are a few observations that you may have missed:

  1. Transaction doesn't even look at the throw_exceptions flag. It behaves the same way regardless, because MySQL++ exceptions might not be the only type being thrown in your program. (e.g. runtime_error from std::vector.)

  2. Given that Transaction does not normally roll back exceptions in the face of a DB error until it dies (again, RAII) that means Transaction would be the wrong place to be looking for error checking methods. The object's dead; you can't call such methods on it. Thus, you're back to checking other longer-lived objects for error results.

Maybe you should try to patch Transaction to make it behave the way you expect, then post the code here for discussion. I suspect that if you try this, you'll get partway into the project and then realize that you're down a blind alley, but who knows, maybe you've got an enlightening idea that could improve MySQL++.

(3) By bruno on 2020-02-27 23:28:02 [link] [source] in reply to 2

In the absence of exceptions, I'm not sure what sort of value you're expecting it to provide.

Any type of other application error, e.g.:

  • Start transaction
  • Read row 1
  • Modify row 1
  • Read row 2
  • Parse row 2 contents and if value X isn't positive, return (i.e.: rollback)
  • Commit

because MySQL++ exceptions might not be the only type being thrown in your program. (e.g. runtime_error from std::vector.)

The code I am working at doesn't make use of exceptions, it uses error codes instead. Runtime exceptions would be treated as a crash in this case. The code creates the connections with 'ne' = true.

Given that Transaction does not normally roll back exceptions in the face of a DB error until it dies (again, RAII) that means Transaction would be the wrong place to be looking for error checking methods. The object's dead; you can't call such methods on it. Thus, you're back to checking other longer-lived objects for error results.

I think that could be done explicitly by the Transaction object, since it contains the connection object. I was thinking on modifying it along the lines of:

std::unique_ptr<Transaction> CreateTransaction(Connection* conn) {
    // Begin the transaction set
    Query q(conn_.query("START TRANSACTION"));
    if (consistent) {
        q << " WITH CONSISTENT SNAPSHOT";
    }
    if (!q.execute()) {  // error code could be extracted from conn
        return nullptr;
    }
    return std::make_unique<Transaction>(*conn);
}

Transaction::Transaction(Connection* conn, bool consistent) :
conn_(*conn), finished_(false) {}

Transaction::~Transaction() {
    if (!finished_) {
        try {
            // even if it fails, on error the transaction will be rollback
            rollback();
        } catch (...) {
            // eat all exceptions
        }
    }
}

bool Transaction::commit() {
    // error code could be extracted from conn
    if (!conn_.query("COMMIT").execute()) {
        return false;
    }
    finished_ = true;
    return true;
}

This way both exception and exception-free usages would be supported. Wdyt?

(4) By Warren Young (tangent) on 2020-02-28 00:42:50 [link] [source] in reply to 3

Parse row 2 contents and if value X isn't positive, return (i.e.: rollback)

The error return you're talking about is already a property of the Result object, thus the frequent use of res in bool context within the samples. For instance, in simple1:

if (mysqlpp::StoreQueryResult res = query.store()) {

that could be done explicitly by the Transaction object, since it contains the connection object

If the error you want to trap is reported by the Connection object, then ask the Connection object, don't make Transaction report it for you. That isn't its job.

std::unique_ptr<Transaction> CreateTransaction()

I see no reason at all to use the heap here. Transaction is a 16-byte object on a typical 64-bit system. Adding smart pointers on top of that is even harder to justify here.

What are you going to do with the smart pointer if you had it, anyway? Keep an instance of it past the end of the call frame so you can ask Transaction what Connection said about the error state of the library? Then all you're doing is delaying the transaction rollback until the final instance is destroyed.

Transaction needs to be single-instance on the stack in order to do its job properly! (For the same reason as for unique_ptr, incidentally.)

bool Transaction::commit()

This seems to be the only necessary part of your changes, but to accept that, we'd have go break the library ABI. That's quite unlikely to happen ever again, since our rules would require that we move to MySQL++ 4.0 for that, and all efforts to get anyone interested in a successor to MySQL++ 3.x have fizzled. (Primarily libtabula.)

If you're willing to work on libtabula, then maybe this change goes there, but I think MySQL++'s ABI is frozen for all practical purposes now.

(5) By bruno on 2020-02-28 04:04:08 [link] [source] in reply to 4

Thanks Warren,

The error return you're talking about is already a property of the Result object, thus the frequent use of res in bool context within the samples. For instance, in simple1

But I would also like to capture the error at the start of a transaction (i.e. the START TRANSACTION statement status).

If the error you want to trap is reported by the Connection object, then ask the Connection object, don't make Transaction report it for you. That isn't its job.

I know that is how the lib exposes errors, but it is easier to me to think about the errors belonging to their entities. Example: a Transaction object error represents the error that happened during a transaction, a Query object error represents the error that happened during a query, and so on (instead of checking the connection object).

I see no reason at all to use the heap here. Transaction is a 16-byte object on a typical 64-bit system. Adding smart pointers on top of that is even harder to justify here.

I was just trying to exemplify a factory function that could return some sort of error status (the nullptr in that case).

This seems to be the only necessary part of your changes, but to accept that, we'd have go break the library ABI.

I understand your concern. In terms of functionality, do you think this is the right approach?

That's quite unlikely to happen ever again, since our rules would require that we move to MySQL++ 4.0 for that, and all efforts to get anyone interested in a successor to MySQL++ 3.x have fizzled. (Primarily libtabula.) If you're willing to work on libtabula, then maybe this change goes there, but I think MySQL++'s ABI is frozen for all practical purposes now.

I think that if you move to a more common repository it could gain traction again and more people would contribute! (like GitHub, just my opinion)

Best regards

(6) By Warren Young (tangent) on 2020-02-28 04:34:39 [link] [source] in reply to 5

But I would also like to capture the error at the start of a transaction (i.e. the START TRANSACTION statement status).

You can probably achieve that by adding a safe bool operator instead, so you can test the Transaction object in bool context before making any queries on the connection.

I'm not sure whether you can do this without adding a data member to the object, again breaking the library's ABI, but it'd be a much better solution than wrapping an object that naturally lives on the stack in an object that must live on the stack which in turn holds the actual object on the heap. Keep it on the stack to begin with!

I was just trying to exemplify a factory function that could return some sort of error status (the nullptr in that case).

Use of nulls to signify errors is usually a bad design.

do you think this is the right approach?

I have no objection to making commit() return false on error. The only question is whether you want to carry the patch locally or work with me to get libtabula into shape so the patch can be upstreamed.

I think that if you move to a more common repository it could gain traction again and more people would contribute! (like GitHub, just my opinion)

The MySQL++ source repo has been mirrored to GitHub for most of a year now. Note the complete lack of PRs.

Almost always when someone complains about the version control system as the reason why contributions don't exist, that is not the actual reason. FOSS predates widespread version control entirely; FOSS didn't begin in 2005 when Git was created.

(7) By bruno on 2020-03-03 05:46:05 [source] in reply to 6

Thanks Warren!

I don't have much time to help with libtabula, sorry.

I am not complaining about fossil, I think that they both have their pros and cons, but GitHub is more commonly used. I think no one contribute with PRs because there is an explicit line in the readme saying to not create any pull requests there!