Fork me on GitHub
#hyperfiddle
<
2023-01-05
>
Dustin Getz17:01:49

@denik websocket reconnection is pushed and tested on mobile safari

4
Dustin Getz17:01:14

Note that if the server has redeployed and the client connects to a newer server version, the reconnection will occur despite the code version mismatch, and this is undefined behavior

Dustin Getz17:01:22

We opened a ticket to deal with this, likely by validating a hash of the program or something like that and asking the user to refresh the page to get the new version. Transparently upgrading through continuous deployments is possible long term but not a priority

denik17:01:25

thank you!! will test asap!

Dustin Getz18:01:36

let me know, i wasn't able to get my phone to sleep to repro your actual issue (for me on mobile safari the websocket stayed alive no matter what i did to my phone, just throttled)

denik19:01:11

will the reconnect survive client/server state

denik19:01:22

I think that was why yam blew up in the past

Dustin Getz19:01:45

the app will reboot. We did implement resync but in testing found a design flaw in Photon, we intend to address it in the next photon iteration (starting now)

Dustin Getz19:01:18

you'll need to put your state in durable atoms, so backed by localstorage, url, database etc

👌 2
denik20:01:02

once there’s an error this change seems to land in a loop where the frontend keeps restarting and retriggering the error

👀 2
Dustin Getz21:01:11

Leo pushed fixes to this

Dustin Getz21:01:27

maybe wait a few days for us to shake out any more bugs - up to you

👌 2
denik21:01:33

okay, I can work with it for now

denik21:01:51

however, the frontend now crashes when the backend errors

denik21:01:00

I know what causes the error in this case. the server is actually fine. it’s one p/server expression calling a function in a p/defn view that errors on certain inputs. it seems that view could error in isolation without blowing up the rest of the stack.

Dustin Getz22:01:13

ok let me think about that

Dustin Getz22:01:45

do you have a try/catch everything at the entry point?

denik22:01:16

yes

(p/defn Main []
  (try
    (let [!path (m/mbx)
          route (decode-path (router/path !path) hf/read-edn-str)]
      (binding [router/Link (router/->Link. !path encode-path)
                dom/node (dom/by-id "root")]

        (p/server
          (views/App. route))))

    (catch Pending _)
    (catch Cancelled e (throw e))
    (catch :default err
      (js/console.error (str (ex-message err) "\n\n" (dbg/stack-trace p/trace)) err))))

(def ^:export main
  #?(:cljs
     (p/boot
       (Main.))))

denik22:01:51

is throwing cancelled a problem?

Dustin Getz22:01:11

you’re throwing cancelled ?

denik22:01:55

what’s the most sensible thing to do?

Dustin Getz22:01:55

i need to follow up with the team about that line

👌 2
Dustin Getz22:01:54

To repro the issue, i tried

(ui/button {::ui/click-event (p/fn [e]
                                   (p/server
                                     ((identity nil)) ; crash
                                     (swap! !x not)))}
      "toggle client/server")
but it doesn't disconnect

Dustin Getz22:01:09

what exception is being thrown in the view that is crashing the server reactor?

denik22:01:47

it’s the other way around: the server is throwing an exception that crashes the client

denik22:01:06

it’s just a nullpointer exception

Dustin Getz22:01:21

in my example i have a NPE on click via (p/server ((identity nil)))

denik22:01:40

I wonder if the issue is that transit can’t serialize the exception?

Dustin Getz22:01:58

i think we send a dummy RemoteError

Dustin Getz22:01:56

I'm not sure what to try next to reproduce

denik22:01:10

and causes a reactor failure

Dustin Getz22:01:52

Following up in public after DMs, this blows up: (comment (encode (Failure. (Remote.)))) at REPL from is a stackoverflow

Dustin Getz22:01:06

this seems obviously an older regression which is getting in the way of understanding the NPE response

Dustin Getz14:01:01

The stackoverflow is fixed

Dustin Getz14:01:03

@denik let's recalibrate - what is the current status, does a NPE in userland cause undesired behavior now that the failure serialization error is fixed?

denik16:01:45

great! just tested and the infinite loop, eventually stackoverflow is fixed

denik16:01:37

hmm though I am getting other Failure serialization errors that I did not get before now

denik16:01:44

now after the initial serialization error every next p/server expr seems to rethrow it