Fork me on GitHub
#hyperfiddle
<
2023-07-30
>
braai engineer11:07:43

This component that shows waiting time on a ticket at second-granularity ran for a while and then caused a 1011 Server process crash:

(defn wait-time->color
  "turn anything older than 2 seconds red"
  [seconds]
  (if (> seconds 2) "red" "blue"))

(defn Date->seconds [date]
  (int (/ (.getTime date) 1000)))

(e/defn Card [tid]
  (e/server
    (let [ticket         (d/entity db tid)
          timestamp      (:ticket/timestamp ticket)
          now-seconds    (int e/system-time-secs)
          waiting-time-s (- now-seconds (Date->seconds timestamp))]
      (e/client
        (dom/div
          (dom/props {:class "card" :style {:background-color (wait-time->color waiting-time-s)}})
          (dom/text "Card"
            " ⏰" waiting-time-s "s"))))))
Haven't reproduced yet, but am I using e/system-time-secs correctly? I see that e/system-time-secs is still millisecond "granularity".

braai engineer11:07:32

reproduced. it crashed again after a few minutes of counting. Electric "v2-alpha-349-ge9996713"

braai engineer12:07:21

Exception:

ERROR hyperfiddle.electric-jetty-adapter: Websocket handler failure
java.io.InterruptedIOException: null
	at org.eclipse.jetty.util.FutureCallback.block(FutureCallback.java:153)
	at org.eclipse.jetty.websocket.common.JettyWebSocketRemoteEndpoint.sendBlocking(JettyWebSocketRemoteEndpoint.java:224)
	at org.eclipse.jetty.websocket.common.JettyWebSocketRemoteEndpoint.sendPing(JettyWebSocketRemoteEndpoint.java:169)
	at hyperfiddle.electric_jetty_adapter$make_heartbeat$cr47371_block_2__47377.invoke(electric_jetty_adapter.clj:20)
	at cloroutine.impl$coroutine$fn__4431.invoke(impl.cljc:60)
	at missionary.impl.Sequential.step(Sequential.java:88)
	at missionary.impl.Sequential$1.invoke(Sequential.java:112)
	at missionary.impl.Sleep.trigger(Sleep.java:63)
	at missionary.impl.Sleep$Scheduler.run(Sleep.java:36)
Caused by: java.lang.InterruptedException: null
	at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.tryAcquireSharedNanos(AbstractQueuedSynchronizer.java:1133)
	at java.base/java.util.concurrent.CountDownLatch.await(CountDownLatch.java:276)
	at org.eclipse.jetty.util.FutureCallback.get(FutureCallback.java:125)
	at org.eclipse.jetty.util.FutureCallback.block(FutureCallback.java:147)
	... 8 common frames omitted
DEBUG hyperfiddle.electric-jetty-adapter: Client disconnected for an unexpected reason. {:status 1011, :reason Server process crash}

braai engineer12:07:09

@dustingetz I had to move e/system-time-secs to the client and it hasn't crashed yet, but not ideal.

Dustin Getz12:07:21

this is unexpected let me review closely

braai engineer12:07:10

No other logs, but it's possible I have old log config. This is my logback.xml:

<!-- Jetty logger config. See  and
ask ChatGPT. `scan` and `scanPeriod` enable hot reload of logger config, leave on! -->
<configuration scan="true" scanPeriod="5 seconds">
    <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
        <encoder>
            <pattern>%highlight(%-5level) %logger: %msg%n</pattern>
        </encoder>
    </appender>

    <root level="WARN">
        <appender-ref ref="STDOUT" />
    </root>

    <logger name="hyperfiddle" level="DEBUG" additivity="false"><appender-ref ref="STDOUT" /></logger>
    <!-- <logger name="hyperfiddle.electric.impl.io" level="INFO" additivity="false"><appender-ref ref="STDOUT" /></logger> -->
    <logger name="app" level="DEBUG" additivity="false"><appender-ref ref="STDOUT" /></logger>
</configuration>

Dustin Getz12:07:55

I think this is a heartbeat issue related to something we fixed this week in master

Dustin Getz12:07:16

Did you blur the tab, such that the tab was not in the foreground while you looked at another tab?

braai engineer12:07:49

let me double check. it is in own window in foreground so should have "focus" but maybe I blurred it

Dustin Getz12:07:06

yeah i suspect the issue is not actually connected to e/system-time-ms at all

Dustin Getz12:07:39

the sleep in the stack trace is related to the websocket heartbeat to keep the connection alive

braai engineer12:07:32

am now running two windows: one focused, one blurred. let's see if blurred breaks

Dustin Getz12:07:15

BTW, be aware that when you stream a clock from the server to the client, that can consume a lot of bandwidth. Here the clock is truncated to 1hz so that should be a lot better than streaming every tick - which can like saturate the wire in theory

👍 2
braai engineer12:07:34

yup, the blurred window died. focused still running

braai engineer12:07:23

can Electric reconnect & resync yet? or force reload?

Dustin Getz12:07:13

i think this is just a bug , we will try to release the fix this week

Dustin Getz12:07:41

resync is blocked on delivery of the next electric version first

braai engineer12:07:33

k so I did the same test but with client side time, and it does not crash

Dustin Getz12:07:07

thanks, that is interesting

braai engineer11:07:15

I am building some cool shit in Electric. Something simple but big is coming soon! I have been waiting for Electric my whole "career". I used to 'mock' stuff in Reagent in DataScript and wasted sooo much time trying to get data sync to server working coz I'm not as smart as @robert-stuttaford, but now I can just outsource my IQ to @dustingetz &@leonoel.

🚀 12
😅 2
braai engineer11:07:13

also @robert-stuttaford how are you not in here?? you need to be tracking Electric.

Dustin Getz11:07:32

any teasers about your thing? 🙂

robert-stuttaford07:07:06

glad to hear you're digging it @U051SPP9Z 😄

robert-stuttaford07:07:33

and thank you for the kind words! 😊

braai engineer12:07:33

Why does -get-system-time-m definition have this signature in args: [& [_]]?

(cc/defn -get-system-time-ms [& [_]] #?(:clj (System/currentTimeMillis) :cljs (js/Date.now)))

Dustin Getz12:07:14

(hyperfiddle.electric/def system-time-ms "ms since 1970 Jan 1" 
  (if (= "visible" dom-visibility-state)
    (new (m/sample -get-system-time-ms <clock))
    (throw (Pending.)))) ; tab is hidden, no clock. (This guards NPEs in userland)

Dustin Getz12:07:32

m/sample calls -get-system-time-ms once per clock tick, m/sample passes the current time, -get-system-time-ms ignores it because it is sampling the system clock by side effect

Dustin Getz12:07:10

<clock is the efficient requestAnimationFrame clock (ticker, really) running at the browser animation rate

braai engineer12:07:45

Does it need to pass the current time to...force an effect? Or just happens to be implemented that way?

Dustin Getz12:07:35

no, this is just type tetris w/ m/sample

braai engineer13:07:25

Why am I getting illegal invocation for this?

(dom/on "click" (e/fn [e]
                              (e/client
                                (let [new-name (js/prompt "Rename" (or (:entity/name entity) ""))]
                                  (if-not (string/blank? new-name)
                                    (e/server
                                      ;; todo auth.
                                      @(d/transact conn [[:db/add eid :entity/name new-name]])
                                      nil))))))

Dustin Getz13:07:42

show error please

Dustin Getz13:07:00

i think that error came from Clojure not Electric

braai engineer13:07:22

The JS prompt does not even show

Dustin Getz13:07:47

does it take a window reference as the first parameter?

Dustin Getz13:07:13

(js/console.log) is (.log js/console)

braai engineer13:07:30

no, but prompt lives on both

Dustin Getz13:07:34

not sure what (js/prompt ) transpiles to

braai engineer13:07:55

js/window.prompt also fails

Dustin Getz13:07:31

i would get this working outside of electric first if we can confirm this is the line that is failing

braai engineer13:07:01

works if I move it out into own fn

braai engineer13:07:08

(defn prompt-rename [qid name]
  #?(:cljs (js/prompt "Rename queue" name)))

Dustin Getz13:07:22

hm ok could be a compiler bug

Dustin Getz13:07:00

you can put the reader conditional on the outside btw

Garrett Hopper19:07:34

Had the exact same issue this morning with js/alert :thinking_face: (But not js/console.log which was strange) It also worked when I wrapped it in its own defn that passed along the string. 🤷

Dustin Getz19:07:51

logged a ticket

🙌 4
Garrett Hopper19:07:52

If I have multiple (side-effecting) functions that I want to run when a single reactive variable changes, is there any way to do so while keeping the functions inline. (I could create an external defn (non-reactive) which would then be called with the arguments, however it'd be nice to inline the calls) E.g.

(defn do-both-things [x y]
  (do-thing x)
  (do-other-thing-also y))
(e/defn Example []
  (let [!x (atom nil) x (e/watch x)]
    (do-both-things x y)))

;; VS

(e/defn Example []
  (let [!x (atom nil) x (e/watch x)]
    (do-when [x] ;; <--- How?
      (do-thing x)
      (do-other-thing-also 1234))))

Dustin Getz19:07:44

you cannot do this today without wrapping it in a clojure lambda. What is the use case?

Dustin Getz19:07:17

((fn [] (f x) (g))) can be inlined in Electric regions, this is what such a macro would do if we supplied it, which we are disinclined to do

Dustin Getz19:07:29

that will rebuild the clojure lambda when x changes and thus call it again, and the clojure lambda has clojure semantics

Garrett Hopper19:07:22

Hmm, that IIFE isn't a bad idea. I don't know that there's a strong use case for it. (In most situations it'd be more desirable to create a external function anyways.) The use case I specifically ran into was some canvas context manipulation logic which requires multiple calls to set up the imperative context prior to the final call which actually depends on the reactive variable.

Dustin Getz19:07:02

Yeah when dealing with foreign interfaces this happens a lot. We intend to provide better idioms here. One major issue with Electric today is that missionary m/ap macro (which is probably the right way to sequence imperative logic), its macroexpansion has a teeny tiny array mutation in Cloroutine way down the stack which is incompatible with Electric's evaluation model

Dustin Getz19:07:57

which we need to fix

Garrett Hopper19:07:30

Interesting :thinking_face: I kinda of expected there'd be some sort of missionary specific implementation that would do this. May have to dig into the m/ap expansion to understand what you're saying about it being incompatible with Electric at the moment.

Dustin Getz19:07:02

m/ap macroexpands to cloroutine which macroexpands to low level array stuff for performance, i believe

Dustin Getz19:07:42

that assumption (of secret internal-only mutation being harmless) suddenly being violated when that macroexpansion occurs inside electric evaluation context and is suddenly reactive

Garrett Hopper19:07:01

Electric functions are themselves just being compiled to missionary flows (?), right?

Garrett Hopper19:07:30

Still trying to connect the dots between my (limited) understanding of missionary and how Electric programs are compiled into it.

Garrett Hopper19:07:25

Err, I'm realizing my mental model was off. I thought any missionary flow could be embedded in Electric by calling new on it. I didn't realize e/def was needed on the m/ap definition.

(e/def example-flow (m/ap "a" "b"))
(e/defn Example []
  (dom/div (dom/text (new example-flow))))

Dustin Getz20:07:02

it’s not, any missionary flow can be “signal awaited” with new, subject to the constraint that the flow has an initial value

👍 2
Dustin Getz20:07:37

m/ap is not supported in electric code however due to the cloroutine issue

👍 4
Dustin Getz20:07:49

you’ll need to wrap in clojure lambda

Garrett Hopper15:08:21

I once again find myself wishing for a React.useEffect style macro that would allow an implicit do to be re-run whenever an input array of reactive vars are changed. (I am open to the possibility of it being a bad idea; I haven't thought about it very deeply yet.) It would be slightly different from just wrapping the logic in a defn, as the body would have access to context variables in the closure without having to explicitly pass them to the function.

Dustin Getz15:08:30

m/observe is react.useEffect except with automatically correct dependency tracking

👀 2
Dustin Getz15:08:06

really the only viable path forward is to make the dependencies explicit in the dag with an IIFE

Dustin Getz15:08:29

the root cause of the pain is dropping out of the immutable information model

Garrett Hopper15:08:49

👍 Thank you 🙂

Dustin Getz15:08:38

oh, note that using m/observe to sequence here involves a clojure lambda, so it accomplishes what the IIFE accomplishes except with more machinery

xificurC15:08:07

> the body would have access to context variables in the closure without having to explicitly pass them to the function are you sure this doesn't work? E.g. this works

(e/defn Foo []
  (let [x 1]
    (#(inc x))))

Garrett Hopper15:08:52

The IIFE would, yes TBH, I'm not sure exactly what I thought would be solved by a macro when I wrote that. Maybe the ability for it to not recreate the function every time a react var changes? I think I'm satisfied that there's nothing clear to be gained here at the moment.

Dustin Getz15:08:34

I would prefer to let the compiler optimize at that layer, we don't optimize much yet but optimizations like avoiding closure allocation can probably be done mechanically

2