edgurgel / verk

A job processing system that just verks! 🧛‍
https://hex.pm/packages/verk
MIT License
723 stars 65 forks source link

Fix: Handle append faulty_nodes in case load more #204

Closed rubikill closed 3 years ago

rubikill commented 3 years ago

Problem

I got this error on my production

** (Protocol.UndefinedError) protocol String.Chars not implemented for {{:with_clause, ["36641929812818644033", "2187482888970067498", "9627219402329306068", "353610110036151689", "33236507393978854108", "19611838561624161266", "30576041652196207300", "1551057097986482091", "42145205601900827183", "29876952421979489883", "37738126232880330471", "24505761782513497057", "17136033922513134781", "1485598031467497790", "17599453943483853695", "3558393621368456405", "616459240496135303", "10169643191668392827", "765868102454266411", "145227747445361820", "24894922613742583604", "31462124922949397927", "518085950846641531", "3829387033188595508", "22009148512253282436", "20947109943320514825", "2211532390129702111", "292284132671991992", "4011903783950186725", "32828455362384227007", "210961816089464794", "39459374272159608664", "18821201871240913943", "30154612842660402327", "11455763694027768228", "20152578363320471161", "9881049053566004972", "38308453783329589584", "30910337762296594148", "9152132193854603092" | {:ok, ["39557249711017784464", "10957392231592429858", "229364207464975306", "3595980527888150297", "11055692083226157890", ...]}]}, [{Verk.Node.Manager, :handle_info, 2, [file: 'lib/verk/node/manager.ex', line: 34]}, {:gen_server, :try_dispatch, 4, [file: 'gen_server.erl', line: 637]}, {:gen_server, :handle_msg, 6, [file: 'gen_server.erl', line: 711]}, {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 249]}]} of type Tuple. This protocol is implemented for the following type(s): Postgrex.Copy, Postgrex.Query, Floki.Selector.PseudoClass, Floki.Selector.Combinator, Floki.Selector.AttributeSelector, Floki.Selector.Functional, Floki.Selector, Decimal, BitString, Atom, NaiveDateTime, URI, Time, Float, List, Date, Version, Integer, Version.Requirement, DateTime

The reason is that the function trying to append a list with a tuple.

Solution

Init the return value with an empty list. Then recursive with an accumulate list

edgurgel commented 3 years ago

Hey thanks @rubikill !

I wonder if we could add a test here where we get {:more more than once? https://github.com/edgurgel/verk/blob/master/test/node/manager_test.exs#L58-L59

rubikill commented 3 years ago

I change the order of the test case

expect(Verk.Node, :members, fn 0, Verk.Redis -> {:more, [dead_node_id], 123} end)
expect(Verk.Node, :members, fn 123, Verk.Redis -> {:ok, [@verk_node_id]} end)

then run the test with master code

➜  verk git:(master) ✗ MIX_ENV=test mix test test/node/manager_test.exs:54
Excluding tags: [:test]
Including tags: [line: "54"]

  1) test handle_info/2 heartbeat when more than 1 node - dead (Verk.Node.ManagerTest)
     test/node/manager_test.exs:54
     ** (WithClauseError) no with clause matching: ["dead-node" | {:ok, []}]
     code: assert handle_info(:heartbeat, state) == {:noreply, state}
     stacktrace:
       (verk 1.7.2) lib/verk/node/manager.ex:34: Verk.Node.Manager.handle_info/2
       test/node/manager_test.exs:79: (test)

Finished in 0.1 seconds
6 tests, 1 failure, 5 excluded

Randomized with seed 716824

and with my fix branch

➜  verk git:(master) ✗ MIX_ENV=test mix test test/node/manager_test.exs:54
Excluding tags: [:test]
Including tags: [line: "54"]

.
Finished in 0.1 seconds
6 tests, 0 failures, 5 excluded

Randomized with seed 385986
edgurgel commented 3 years ago

Gotcha! Thanks! Will check it out later today 👌