search

phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6k stars 906 forks source link

Refactor error handling #3280

Open SteffenDE opened 1 month ago

SteffenDE commented 1 month ago

@chrismccord this definitely needs some work (and probably from you).

References #3257. References https://github.com/phoenixframework/phoenix_live_view/pull/3279.

Summary of the current state:

  1. when a child LV join fails we call reloadWithJitter with the child LV
  2. reloadWithJitter calls disconnect on the socket
  3. disconnect destroys all child views
  4. the timeout in reloadWithJitter runs and checks if the view is destroyed -> yes -> nothing happens, because we just return
  5. we are stuck

I think we’ll need to first define the expected behavior, as this is a little bit tricky. If I remove the call to disconnect, the first thing I noticed is that the phoenix channel itself tries to rejoin separately from the LV code (https://github.com/phoenixframework/phoenix/blob/69685f783bc60d2a124fc2fb48028b065bc24b22/assets/js/phoenix/channel.js#L45-L48). So we need to keep the Phoenix Socket retries in mind.

I think it should probably be like this: A child LV that crashes (on join or whenever) should be rejoined without affecting any other LiveView on the page. We could keep track of the number of failed joins similar how we keep track of consecutive reloads. If we reach a number of consecutive child joins with no success we should do a hard refresh (this will then of course kill all other LiveViews as well). After we tried a hard refresh and the child LV still fails, we give up.

This PR tries to implement what was described above. As I'm not 100% sure I've grasped everything happening from Phoenix.Socket to the different considerations for LV, this is still a draft. Some TODOs are in the code. I think the tests are good to have - even if the JS changes are thrown away, those can be kept and adapted accordingly.