Fix socket not reconnecting - #300
Conversation
172d3d8 to
3cb6cd8
Compare
3cb6cd8 to
b6b557a
Compare
|
Converted this to draft for now, I am still evaluating what else can be done so notifications won't get lost |
cdddfc0 to
7c20fb5
Compare
74a62ba to
8a9a522
Compare
8a9a522 to
f3f067b
Compare
|
This got way bigger then expected but I think this is pretty solid now. This now handles mint errors while trying to push/reconnect to the different services. I extracted a bit of adapter logic that applies to both apns + fcm to a helper. the ADM adapter does not use very much of it because it has a different behaviour |
| Supervisor.child_spec({Pigeon.DispatcherWorker, opts}, id: index) | ||
| end | ||
|
|
||
| children = [ |
There was a problem hiding this comment.
Hey, thank's for the fix !
Just a remark on this line, since BackoffWorker is a named GenServer, we cannot start 2 of it. But as we can start multiple Dispatcher (one for APNS and one for FCM for example) the application cannot start :
** (Mix) Could not start application app_web: Veezu.AppWeb.Application.start(:normal, []) returned an error: shutdown: failed to start child: Veezu.Messaging.APNS
** (EXIT) shutdown: failed to start child: Pigeon.BackoffWorker
** (EXIT) already started: #PID<0.625.0>
Has there is no need to have one BackoffWorker per dispatcher, the fix is to start the BackoffWorker in the main application supervisor than per dispatcher
|
hi, im using your fork in my project hoping it will fix the problem but i still get this: can i get an idea on why this happen ? |
This handles error when the server closes connections.
The second commit implements a check for a open connection but it is not very resilient yet.
If the reconnect fails, the notification get's lost, either there needs to be some recovery mechanism or the response handler needs to be called.
#299