Client Error Handling needs improvement

[Nodstuff]

Client errors get returned as protocol.Error which is located in the internal package, this means that users of the library are unable to use the error type to assert on information contained within it, such as Code.

Also, is it possible to map and return the SQLite3 Error codes as seen in github.com/mattn/go-sqlite3's sqlite3.Error? this would make error handling much more user friendly.

Lastly, using the RETURNING statement (autogenerated with new versions of sqlboiler) renders any error returned from dqlite unusable and breaks our applications error handling. Our choices are pin to an old version of sqlboiler or look into dropping dqlite for another project, neither of these are ideal solutions so it would be great if we could have some kind of user-friendly error handling with exposed constants so that we can explicitly check errors returned.

[freeekanayaka]

Client errors get returned as protocol.Error which is located in the internal package

There is a driver.Error type alias here. Is that enough?

this means that users of the library are unable to use the error type to assert on information contained within it, such as Code.

I'm not sure I understand what you mean here exactly, care to elaborate?

Also, is it possible to map and return the SQLite3 Error codes as seen in github.com/mattn/go-sqlite3's sqlite3.Error? this would make error handling much more user friendly.

It is now possible to use the go-dqlite package without using or depending on go-sqlite3. So if you mean returning the exact same error objects/types defined in go-sqlite3, I'm not sure we'd want that.

In which way would that make error handling more user friendly? Maybe there's a way to achieve the user-friendliness you want, without depending on go-sqlite3.

Note also that the errors return by go-dqlite are a superset of the ones returned by go-sqlite3.

Lastly, using the RETURNING statement (autogenerated with new versions of sqlboiler) renders any error returned from dqlite unusable and breaks our applications error handling. Our choices are pin to an old version of sqlboiler or look into dropping dqlite for another project, neither of these are ideal solutions so it would be great if we could have some kind of user-friendly error handling with exposed constants so that we can explicitly check errors returned.

We have https://github.com/canonical/go-dqlite/issues/192 tracking that, I think the discussion should be moved there.

[freeekanayaka]

Note that a third option regarding the RETURNING issue would be to add a new dqlite driver to sqlboiler, which accounts for dqlite-specific semantics.

As mentioned in the other issue though, I think that fixing this in dqlite would be indeed better.

[Nodstuff]

There is a driver.Error type alias here. Is that enough?

Yes that works actually, thank you!

It is now possible to use the go-dqlite package without using or depending on go-sqlite3. So if you mean returning the exact same error objects/types defined in go-sqlite3, I'm not sure we'd want that.

In which way would that make error handling more user friendly? Maybe there's a way to achieve the user-friendliness you want, without depending on go-sqlite3.

Note also that the errors return by go-dqlite are a superset of the ones returned by go-sqlite3.

No, what I meant was having some error constants like in that package, i.e. sqlite3.ErrConstraintForeignKey, that makes it much easier to use the client library as errors can be explicitly handled, not that there should be a dependency on that package.

The issue I am seeing now is that I am getting back a Code of 3 for a constraint violation, which doesn't make any sense to me and there is no constant or docs to look up for me to be able to write up something to handle dqlite error codes.

Note that a third option regarding the RETURNING issue would be to add a new dqlite driver to sqlboiler, which accounts for dqlite-specific semantics.

I will look into this, it may be necessary for us in the short term.

As mentioned in the other issue though, I think that fixing this in dqlite would be indeed better.

That would be ideal tbh.

Thank you for your help with this!

[freeekanayaka]

No, what I meant was having some error constants like in that package, i.e. sqlite3.ErrConstraintForeignKey, that makes it much easier to use the client library as errors can be explicitly handled, not that there should be a dependency on that package.

The issue I am seeing now is that I am getting back a Code of 3 for a constraint violation, which doesn't make any sense to me and there is no constant or docs to look up for me to be able to write up something to handle dqlite error codes.

Ok, I see. Yes, introducing constants like the ones go-sqlite3 has might be a good idea.

Regarding the codes, they should be exactly the same as SQLite ones, plus a few additional ones which are dqlite-specific.

If that's not the case, there might be a bug.

Note that a third option regarding the RETURNING issue would be to add a new dqlite driver to sqlboiler, which accounts for dqlite-specific semantics.

I will look into this, it may be necessary for us in the short term.

It shouldn't be too hard, it's basically copy-pasting the existing sqlite3 driver and amend it as needed.

As mentioned in the other issue though, I think that fixing this in dqlite would be indeed better.

That would be ideal tbh.

Agreed.

Thank you for your help with this!

You're welcome!

[freeekanayaka]

No, what I meant was having some error constants like in that package, i.e. sqlite3.ErrConstraintForeignKey, that makes it much easier to use the client library as errors can be explicitly handled, not that there should be a dependency on that package. The issue I am seeing now is that I am getting back a Code of 3 for a constraint violation, which doesn't make any sense to me and there is no constant or docs to look up for me to be able to write up something to handle dqlite error codes.

Ok, I see. Yes, introducing constants like the ones go-sqlite3 has might be a good idea.

Note that if you don't mind depending on go-sqlite3 you can already use the constants defined there and compare them to the codes you get.

[Nodstuff]

Note that if you don't mind depending on go-sqlite3 you can already use the constants defined there and compare them to the codes you get.

That is interesting, what I am seeing at the moment is a very unusual 3 instead of the expected 19 for constraint violation. Do you think this is caused by the RETURNING issue? the returned Msg in the error is just "id".

I will look into the sqlboiler driver, I found the code that I need to remove in the .tpl so that might be a quick win.

If the strange error code is indeed due to the RETURNING issue then adding a driver should resolve that and then I should be able to use the constants from go-sqlite3 until/if they are added to go-dqlite, that would make things a lot cleaner and safer in our application logic.

Thank you so much! I will close this issue if you have no objections

[freeekanayaka]

Note that if you don't mind depending on go-sqlite3 you can already use the constants defined there and compare them to the codes you get.

That is interesting, what I am seeing at the moment is a very unusual 3 instead of the expected 19 for constraint violation. Do you think this is caused by the RETURNING issue? the returned Msg in the error is just "id".

That sounds indeed like a bug that might be caused by the use of RETURNING, but to be sure you'd need to provide some sample code to reproduce the issue, or give us more details.

I will look into the sqlboiler driver, I found the code that I need to remove in the .tpl so that might be a quick win.

Sounds good.

If the strange error code is indeed due to the RETURNING issue then adding a driver should resolve that and then I should be able to use the constants from go-sqlite3 until/if they are added to go-dqlite, that would make things a lot cleaner and safer in our application logic.

Thank you so much! I will close this issue if you have no objections

It might be worth keeping it open until the situation about the "weird" code being returned is clarified. But if you find workarounds that are fine for you, feel free to close this issue.

[cole-miller]

@Nodstuff I'll use this issue to track the feature request for exporting named error constants from go-dqlite, which I agree is reasonable and that we should be able to do easily. Please do open a separate issue for this part, if you're seeing it consistently:

That is interesting, what I am seeing at the moment is a very unusual 3 instead of the expected 19 for constraint violation. Do you think this is caused by the RETURNING issue? the returned Msg in the error is just "id".

[cole-miller]

@Nodstuff, another note on this:

That is interesting, what I am seeing at the moment is a very unusual 3 instead of the expected 19 for constraint violation.

3 is indeed unexpected, but you should be aware that even without bugs dqlite will not return SQLITE_CONSTRAINT = 19 for all constraint violations, because we enable extended result codes for every connection. You can check for all kinds of constraint violations in an extended-result-code-aware way like this:

if err.(driver.Error).Code&((1<<8) - 1) == 19 {
        // ...
}

[Nodstuff]

I'll use this issue to track the feature request for exporting named error constants from go-dqlite, which I agree is reasonable and that we should be able to do easily

Awesome! Thank you so much @cole-miller

dqlite will not return SQLITE_CONSTRAINT = 19

Yeah sorry I was just using it as a lazy example rather than looking up the extended codes as I was typing 😄