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

Default Cassandra Read Body Error Message #108

Merged
merged 4 commits into from Oct 6, 2017

Conversation

Projects
None yet
2 participants
@msindwan
Copy link
Contributor

msindwan commented Oct 5, 2017

We recently encountered an error where the translation was not found for a particular cassandra error. We speculated that this was a write failure and subsequently added it to the list of error codes as per the cassandra protocol. In addition, we added a safe guard for error translations to gracefully handle unsupported error codes.

@thibaultcha

This comment has been minimized.

Copy link
Owner

thibaultcha commented Oct 5, 2017

Oh nice! It seems like WRITE_FAILURE was added in binary protocol v4 (https://github.com/apache/cassandra/blob/trunk/doc/native_protocol_v4.spec#L1106) - while this library was originally developed against the v3. Thanks for the catch!

I am currently in the process of improving the CQL implementation quite a lot (before improving the high-level interfaces). I believe we are having some issues with CI due to third-party dependencies, and those will have to be resolved before I can merge this. Will try to do so as soon as possible!

Thanks again!

@thibaultcha
Copy link
Owner

thibaultcha left a comment

Thank you! Will you address the few issues here? Also, if you rebase on master, the Travis-CI build should pass in your PR. Thanks!

-- If the translation is not found, return a formatted string
-- with the error code for convenience.
if error_translation == nil then
error_translation = string.format(

This comment has been minimized.

@thibaultcha

thibaultcha Oct 6, 2017

Owner

We should be using the localized version of string.format (in the fmt local) instead here.

-- with the error code for convenience.
if error_translation == nil then
error_translation = string.format(
'UNSUPPORTED ERROR (code=%d)', code or ERRORS.UNKNOWN)

This comment has been minimized.

@thibaultcha

thibaultcha Oct 6, 2017

Owner

I don't think we should use the %d modifier here - using fmt("0x%x", code) will print the error code in the same format the binary protocol specification refers to it.

SYNTAX_ERROR = 0x2000,
UNAUTHORIZED = 0x2100,
INVALID = 0x2200,
CONFIG_ERROR = 0x2300,
ALREADY_EXISTS = 0x2400,
UNPREPARED = 0x2500,
UNKNOWN = -1,

This comment has been minimized.

@thibaultcha

thibaultcha Oct 6, 2017

Owner

I don't think this constant is necessary. The binary protocol guarantees us we will be receiving an error code already.

This comment has been minimized.

@msindwan

msindwan Oct 6, 2017

Author Contributor

Nice! Will remove then

@thibaultcha thibaultcha merged commit a7b1229 into thibaultcha:master Oct 6, 2017

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.2%) to 94.614%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thibaultcha

This comment has been minimized.

Copy link
Owner

thibaultcha commented Oct 6, 2017

Merged, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment