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

ArgumentError when decoding an int > 134_217_727 #29

Closed
tr00gle opened this issue May 20, 2020 · 4 comments
Closed

ArgumentError when decoding an int > 134_217_727 #29

tr00gle opened this issue May 20, 2020 · 4 comments

Comments

@tr00gle
Copy link

tr00gle commented May 20, 2020

Hello! I'm getting an exception from the library when trying to decode nine digit integers greater than or equal to 140_000_000. I've pasted the exception and its trace below:

16:35:46.379 [error] GenServer #PID<0.516.0> terminating
** (ArgumentError) argument error
    (avro_ex) lib/avro_ex/decode.ex:220: AvroEx.Decode.variable_integer_decode/3
    (avro_ex) lib/avro_ex/decode.ex:47: AvroEx.Decode.do_decode/3
    (avro_ex) lib/avro_ex/decode.ex:139: anonymous fn/3 in AvroEx.Decode.do_decode/3
    (elixir) lib/enum.ex:1940: Enum."-reduce/3-lists^foldl/2-0-"/3
    (avro_ex) lib/avro_ex/decode.ex:138: AvroEx.Decode.do_decode/3
    (avro_ex) lib/avro_ex/decode.ex:11: AvroEx.Decode.decode/2
    (transactional_messaging_service) lib/avro/decoder.ex:9: Avro.Decoder.decode_datum/1
    (transactional_messaging_service) lib/transactional_messaging_service/kafka_consumer.ex:17: anonymous fn/2 in TransactionalMessagingService.KafkaConsumer.handle_message_set/2
Last message: :timeout

Here's the pertinent snippet of the .avsc schema file in question:

  "fields": [
    {
      "name": "id",
      "type": "int"
    },

I've verified that 130_000_000 can be successfully decoded, but 140_000_000 cannot be successfully decoded.

There are a few things I'm not quite getting, and they're probably a result of my admitted lack of deep elixir debugging experience:

  1. I'm piping the result of AvroEx.decode into a function that matches against either {:ok, decoded_message} and a default clause, but these exceptions are not being handled by the default function clause. Do you know why that might be?
  2. After poring over the source of this library, I noticed that this line is testing five million as the upper bound for an int type, but the avro spec expects any 32-bit signed integer, which goes up to 2_147_483_647, a number much larger than where I'm having trouble. What was the reasoning for the tests to be written this way?
  3. I forked and cloned the repo, but I cant seem to run the test suite because there is a compilation error in field.ex. I'm not experienced with specs or changesets, so I'm not really sure what to fix here.

I'd love to help solve this issue in any way I can, so please let me know how I can be a helpful collaborator and contributor. Thank you so much in advance!

@tr00gle
Copy link
Author

tr00gle commented May 21, 2020

Disregard Question 3: I commented out the @spec and @spec_match and ran the tests as is in the source: they work.

Then, I changed 5_000_000 to 500_000_000 and received the argument error again:

  1) test decode (primitive) integer (AvroEx.Decode.Test)
     test/decode_test.exs:22
     ** (ArgumentError) argument error
     code: assert {:ok, 500_000_000} = @test_module.decode(schema, big)
     stacktrace:
       (avro_ex 1.1.0) lib/avro_ex/decode.ex:238: AvroEx.Decode.variable_integer_decode/3
       (avro_ex 1.1.0) lib/avro_ex/decode.ex:64: AvroEx.Decode.do_decode/3
       (avro_ex 1.1.0) lib/avro_ex/decode.ex:12: AvroEx.Decode.decode/2
       test/decode_test.exs:33: (test)

@tr00gle
Copy link
Author

tr00gle commented May 22, 2020

Ok, so after a bunch more research, it seems that the threshold is 134_217_727.

134_217_727 passes, but 134_217_728 fails. What are the constraints here that it's written this way? Would it be possible to catch these argument errors and treat them as type long automatically? I'm not sure what the potential ramifications are here, but I'm happy to research/do whatever I can to assist.

@tr00gle tr00gle changed the title ArgumentError when decoding an int >= 140_000_000 ArgumentError when decoding an int > 134_217_727 May 22, 2020
@tr00gle
Copy link
Author

tr00gle commented May 26, 2020

After more research, and studying the avro-official decoding/encoding implementations in ruby and javascript, and it seems like the implemented logic in this library is pretty similar, except swapping while loops for recursive calls. A key difference is that the ruby implementation is that the integer-related logic delegates to the same logic for type long, and I wonder if that might be helpful here?

Also, the inability to decode integers that have an absolute value larger than 134_217_727 seems like an unexpectedly low threshold still. A few more follow up questions:

  1. Since tests tend serve as a form of documentation, would it be appropriate to update the test cases for decoding integers to use the value 134_217_727 instead of 5_000_000? I have a branch and commit ready to go, but if it's not something we want, I won't open a PR.
  2. Should we update the tests for type long accordingly as well?
  3. Again, these numbers aren't particularly large. Has nobody else run into these issues either?

pnezis added a commit to pnezis/avro_ex that referenced this issue Jul 28, 2020
Change the implementation of variable `integer` and `long` decoding
based on the `klarna/erlavro` erlang implementation. This fixes the
issue of not decoding correctly integers above 134_217_727.

Fixes beam-community#29
@tr00gle
Copy link
Author

tr00gle commented Sep 29, 2020

NICE!

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

1 participant