Strange {:error, %ArgumentError{message: "argument error"}} about avrora HOT 12 CLOSED

I've had the same problem, had to use a internal fork from a previous version.

from avrora.

@fxn Hey, thanks for this usecase example. I will take a look 🔎

from avrora.

I have simplified the reproduction steps. Channels are not necessary, it happens with controllers too.

The repository with the example app has been updated. WebSockets are removed, and there is a vanilla PageController that just encodes. The test suite of that contoller shows the problem. It has two tests that make requests to the same action. They pass individually, but do not pass together.

% mix test test/avrora_issue_web/controllers/page_controller_test.exs  
{:ok,
 <<79, 98, 106, 1, 3, 134, 2, 20, 97, 118, 114, 111, 46, 99, 111, 100, 101, 99,
   8, 110, 117, 108, 108, 22, 97, 118, 114, 111, 46, 115, 99, 104, 101, 109, 97,
   202, 1, 123, 34, 110, 97, 109, 101, 115, 112, 97, 99, 101, ...>>}
.{:error, %ArgumentError{message: "argument error"}}


  1) test 1 (AvroraIssueWeb.PageControllerTest)
     test/avrora_issue_web/controllers/page_controller_test.exs:4
     ** (MatchError) no match of right hand side value: {:error, %ArgumentError{message: "argument error"}}
     code: get(conn, "/")
     stacktrace:
       ...

from avrora.

@fxn Good news, thanks to provided test case, I've manage to find out where is the problem and I'm working on a fix. The bad news are – it's not related to tests and it's a general issues.

TLRD;

I didn't understand how long and when ETS will be removed, now I know.

Complete answer:

It turn out that ETS belongs to the process where it was created, so in the example you create first test case encodes the schema, schema get parsed and ETS table created and bound to the ... (ta-ba-dums) .... process of a Phoenix controller ... which will die and another test case will run, BUT Avrora.Storage.Memory are sharing state between those 2 test cases and have a reference to already non-existing ETS table because it was cleaned when Phoenix process exists.

Hence this behaviour easily appear on a normal conroller flow, just request the same resource twice.

The fix will include 2 things:

1. All the schema ETS tables will be re-owned by a Avrora.Storage.Memory as the main demanding module (or similar)
2. A test helper to remove state sharing will be added

from avrora.

Thanks!

from avrora.

@fxn @tarcisiozf Hey guys, I've finished the new version and here is a branch #22

It will fix an issue with or without test modifications and the difference is only do you want to share state or not (of course not). Here is an example of what will happen in case of the ETS tables gone:

iex(1)> Avrora.encode(%{"login" => "fxn"}, schema_name: "com.example.User")
[debug] reading schema `com.example.User` from the file /Volumes/SSDCard/Development/github/avrora_issue/priv/schemas/com/example/User.avsc
{:ok,
 <<79, 98, 106, 1, 3, 134, 2, 20, 97, 118, 114, 111, 46, 99, 111, 100, 101, 99,
   8, 110, 117, 108, 108, 22, 97, 118, 114, 111, 46, 115, 99, 104, 101, 109, 97,
   202, 1, 123, 34, 110, 97, 109, 101, 115, 112, 97, 99, 101, ...>>}

iex(2)> Avrora.encode(%{"login" => "fxn"}, schema_name: "com.example.User")
{:ok,
 <<79, 98, 106, 1, 3, 134, 2, 20, 97, 118, 114, 111, 46, 99, 111, 100, 101, 99,
   8, 110, 117, 108, 108, 22, 97, 118, 114, 111, 46, 115, 99, 104, 101, 109, 97,
   202, 1, 123, 34, 110, 97, 109, 101, 115, 112, 97, 99, 101, ...>>}

iex(3)> Process.exit(Process.whereis(Avrora.ETS), [])
true

iex(4)> Avrora.encode(%{"login" => "fxn"}, schema_name: "com.example.User")
[debug] reading schema `com.example.User` from the file /Volumes/SSDCard/Development/github/avrora_issue/priv/schemas/com/example/User.avsc
{:ok,
 <<79, 98, 106, 1, 3, 134, 2, 20, 97, 118, 114, 111, 46, 99, 111, 100, 101, 99,
   8, 110, 117, 108, 108, 22, 97, 118, 114, 111, 46, 115, 99, 104, 101, 109, 97,
   202, 1, 123, 34, 110, 97, 109, 101, 115, 112, 97, 99, 101, ...>>}

Any feedback is appreciated 🤝

from avrora.

Thanks for working on this :), my test suite passes with this branch.

I am not familiar with the implementation, but let me ask you a question: Is the memory storage an internal type of storage that acts as a cache to avoid hitting the registry or file system? Why isn't that GenServer enough to address this?

from avrora.

@fxn Technically yes, Avrora.Storage.Memory is just an in-memory cache. But it follows the same behavior as all other Avrora.Storages. And if we want to stick to 1 owner for all :ets tables, then Avrora.Schema as the module who generates those tables should transfer ownership.

And here I have an OOP dilemma.

Avrora.Schema is a stupid module that just knows how to parse JSON and generate the structure of himself. And it should not know that some ownership exists or that it should be granted (also some Process.whereis(...) will appear, which mean I don't control my own modules).

At the same time if I want to add ownership to Avrora.Storage.Memory it should accept a grant and handle that message, which looks odd from the responsibilities perspective of Storage behavior.

Hence I decided to add the ETS host process which you can ask to create a new table and at the same time, it will be automatically a parent for them.

@fxn you think it's overcomplicated?

from avrora.

@Strech I don't know! :) I'll read the source code more in detail to understand it better.

from avrora.

@fxn Cool will be great to have another pair of eyes. I will release a new version then, and if you will have any suggestions, I can apply them in a next iteration 👍

from avrora.

@Strech perfect, thanks for the fix!

from avrora.

@fxn @tarcisiozf Guys, release 0.9.0 is out! Thanks for your participation 👏

from avrora.