This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2018-03-07
Channels
- # aleph (4)
- # arachne (2)
- # aws (2)
- # beginners (42)
- # boot (4)
- # cider (47)
- # cljs-dev (352)
- # clojure (278)
- # clojure-dusseldorf (6)
- # clojure-italy (6)
- # clojure-russia (1)
- # clojure-spec (15)
- # clojure-uk (94)
- # clojurescript (197)
- # community-development (34)
- # cursive (3)
- # data-science (1)
- # datomic (64)
- # emacs (3)
- # figwheel (16)
- # fulcro (7)
- # hoplon (5)
- # jobs (3)
- # luminus (3)
- # mount (2)
- # nyc (1)
- # off-topic (31)
- # onyx (22)
- # parinfer (1)
- # protorepl (7)
- # re-frame (9)
- # reagent (61)
- # ring-swagger (3)
- # shadow-cljs (149)
- # spacemacs (18)
- # specter (4)
- # timbre (1)
- # unrepl (38)
- # vim (17)
- # yada (14)
Uh, oh, when I pasted that image it indicated: "Your file was uploaded — it’s safe and sound in Slack. Unfortunately your workspace doesn’t have any storage space left. To get more space, you can upgrade to a paid account or delete some of your older files."
There is a patch covering the src="/main.js"
in the rendering above in https://dev.clojure.org/jira/browse/CLJS-2624
^ Actually, you need src="/main.js"
or src="main.js"
to make things work. Not sure if there is any correct way to address this, other than to leave it as is.
Three pieces of feedback received regarding the template: - Make the DOCTYPE declaration in lower-case - Omit the script type tag - Use `<textarea/> to facilitate select / copy I have no opinion on these.
@dnolen Patch with tests added for the :npm-deps {"/path/to/tarball.tgz" "..."}
feature: https://dev.clojure.org/jira/browse/CLJS-2622
@jannis so they only difference is that npm install
is called without version
I wonder how much is makes sense to try to support different cases with :npm-deps
, I would maybe just remove the whole option and make everyone use package.json
Until we do that, this feels like a feature worth adding to support e.g. unpublished packages.
Or we could just document "if you need to do this, use package.json or call npm install yourself"
Perhaps newly-entered https://dev.clojure.org/jira/browse/CLJS-2625 is related to the browser races David was fighting with yesterday
@dnolen my comments yesterday about the browser REPL were spurred by my attempts to use the Browser REPL to work on a more isolated figwheel, I was at first pleasantly surprised and hopeful, everything worked! I was getting feedback in the browser, and my work was clipping along nicely but then after 15-30 minutes of use the Browser REPL connection died 😞. Then in another session the watcher inexplicably died. This was ultimately very disheartening. As this issue has persisted from the very early days of CLJS.
@mfikes we don’t fully wait, but I’m less concerned about that, I think I have an idea (heartbeat thing) which might address a bunch of things at once
Oh, so that points to a problem using a long-lived connection. That's unexplored by me.
@bhauman heh, I think we’re on the same page here - like I said I’d like to actually start using browser REPL on stuff so these kind of problems can get sorted out
Yeah my immediate reaction was: might this be addressed by automatic reconnects? https://dev.clojure.org/jira/browse/CLJS-1147
well my gut says the protobuffer stuff is fragile in general and that HTTP polling is simpler and more robust
It’s complicated (because of the extra layer of async), but it should work. But yeah, I think removing dynamic vars from the server and ajax polling might be better.
I personally haven't found any resources that would help us include websocket support minimally
Perhaps prior / usable art here https://github.com/tomjakubowski/weasel
Even w/ it, wss will be a hassle, right? Almost seems easier to provide instructions for running over SSL.
https://github.com/TooTallNate/Java-WebSocket/blob/master/src/main/example/SSLServerExample.java
traced to here: https://github.com/eventmachine/eventmachine/blob/master/lib/em/connection.rb#L130
http://www.rubydoc.info/github/eventmachine/eventmachine/EventMachine/Connection:start_tls
I'm thinking with this example, you can gen a cert in process https://github.com/TooTallNate/Java-WebSocket/blob/master/src/main/example/SSLServerExample.java#L44
I mean, that looks pretty legit. At least worth a shot. Might still require the user to override alerts in the browser.
And the main upside vs ajax is, this maybe gives us ssl out of the box? It sounds like we may be able to pull off ajax+https via the same trick.
We could fire up 2 http servers on different ports to get that same isolation though, right?
other option which I also think is fine and less churn is supporting the old interface, and refactoring it a bit and then doing the WebSocket one as the recommended way
@potetm I would think all threads use the same websocket server, and we tag the messages
Yeah, I meant, as an alternative to websockets, we could have 2 webservers: one that serves static content, one that handles the repl. This is another way to break up the repl from the static serve.
@dnolen not sure if it was discussed before, but it would be helpful to clarify the order of command line flags in cljs.main
, perhaps in Quick Start or even in --help
?
@roman01la we do
You are right. If only it could shout in the face 🙂 I guess I’m not the only one who will miss this line in help
@roman01la there’s not really much we can do because -m
-r
-c
must come last
Interesting. enhanced-resolve
(which we’re using in @cljs-oss/module-deps
) always takes what’s in the main entry fields, regardless of what extensions
or moduleExtensions
you give it through its resolver configuration. Those just define preference/order in case there is no package.json
.
I wonder if I can write a basic plugin for it that catches resolved .mjs
files and translates them to .js
if possible.
Here is a browser REPL stress test that provokes a failure fairly quickly. If we get it to survive for more than a few seconds, then it can also serve as a way to simulate a user using a long-lived session. https://gist.github.com/mfikes/765ac8e43d70b743e3bfbd02ff4703a1
Within a few seconds at most I get
[Error] Failed to load resource: The network connection was lost. (localhost, line 0)
Yeah, let me try to figure out how to get some parameters that would launch Figwheel. That shouldn't be to hard I suppose.
In that test, it does a console log of how many iterations it has done. Browser REPL makes it to 15 or 20. Figwheel is already past 1000 for me.
Updated the gist with Figwheel instructions https://gist.github.com/mfikes/765ac8e43d70b743e3bfbd02ff4703a1
FWIW, the browser REPL survives that test for me on a faster box, so there is perhaps some CPU-related race?
a processing speed difference of that magnitude shouldn't cause connection corruption??
@mfikes but when you say browser REPL “craps out” you mean there’s an exception? or it locks up?
In the JavaScript console, it logs 10 or 15 iterations, and then prints
[Error] Failed to load resource: The network connection was lost. (localhost, line 0)
and stops.No... but I can try to ascertain more about precicely how it is failing. Perhaps the simple web server we are running craps out in a way that says something...
Yeah, I'll get to the bottom of it; it is trivial to repro so I'll have more info shortly
The amount of time it can run before craping out is heavily dependent on the parameters you put in the Thread/sleep
call in the stress test... if you use larger values, things work OK
we are, of course, assuming that the stress test is revealing the longevity issue, which occurs in the absence of constant interaction
@dnolen Yeah, perhaps it is a "livelock" (or promise delivery failure). Anyway, you can see that in the last stack here https://gist.github.com/mfikes/71db4c4a50f314824ac9dd0892d5c134
Checking on my mobile, it looks like browser/send-for-eval
is starved waiting for the promise to a connection to be delivered
I'm wondering if it is just a downstream derailment after the browser side indicates the network connection was lost. I'm trying a test involving commenting out some of the .close
calls in cljs.repl.server
to see if something is prematurely closing. Dunno.
Repro is even easier if you do cat repl-input.txt | clj -A:cljs
where the repl-input.txt
has a lot of lines with keywords to evaluate
Yeah, I wish I had taken better notes, but perhaps my 20% failure rate claim came from working on the 2009 iMac, where on the 2013 iMac things are solid
Cool. I'll check around that code. If you think of any speculative changes, happy to try them on the 2009 iMac.
Eh. So falling back to .js
in module_deps.js
works but of course the inter-package dependencies that involve .mjs
files are still broken—that’s Closure’s part.
I’m not sure how likely a patch that implements the same fallback in Closure would be accepted.
Yeah I am exploring that part as well, as I wrote above..do you even need promises there?
just a note: keep in mind that currently you can reload the browser and the connection is maintained
@dnolen @anmonteiro What do you reckon about the above? Looks like the .mjs -> .js
fallback would have to be implemented in Closure as well to keep things in sync. Is it realistic at all to get that kind of change into Closure?
Right now, with an enhanced-resolver
plugin, both graphql
and iterall
index fine by using only .js
files but the dependencies between graphql and iterall sources are still broken (the graphql source files have module$iterall
in a few places and no goog.require('module$...$iterall)
at all).
@mfikes I think queue + a new thing from the client :accept
would make this work for all cases
I'm currently trying a less aggressive change to simply retain the current code but to properly do things atomically in cljs.repl.server/connection
and cljs.repl.server/set-connection
(in other words properly use the atom instead of deref, and then swap on it, leaving a window for corruption to sneak in)
@mfikes lol I am doing the same 😄
the browser sends two requests on two different ports
The code just looks like it admits a race, and it might be an easy, less risky change to see what that does for this failure mode
@richiardiandrea you don’t want different ports
(I don't understand the bigger picture of all of the code, and it sounds like there is more to be done, but looking at just those two functions in isolation, they could be made thread-safe, so just gonna try an experiment.)
yeah David, I know 😄 I am just reporting what is happening, anyways...let me keep digging, interesting stuff 😄
this is what happens here:
Conn requested
Promise a connection
Set connection #object[java.net.Socket 0x1e5a28e3 Socket[addr=/0:0:0:0:0:0:0:1,port=48686,localport=9000]]
Queue nil
Deliver a promised connection #object[java.net.Socket 0x1e5a28e3 Socket[addr=/0:0:0:0:0:0:0:1,port=48686,localport=9000]]
Set connection #object[java.net.Socket 0x913d1b8 Socket[addr=/0:0:0:0:0:0:0:1,port=48688,localport=9000]]
Queue nil
Conn requested
Deliver #object[java.net.Socket 0x913d1b8 Socket[addr=/0:0:0:0:0:0:0:1,port=48688,localport=9000]]
Set connection #object[java.net.Socket 0x2e4e0bcb Socket[addr=/0:0:0:0:0:0:0:1,port=48738,localport=9000]]
Queue #object[clojure.lang.PersistentQueue 0x61b4fa49 clojure.lang.PersistentQueue@913d1d7]
Conn requested
Promise a connection (waiting)
which evals bootstrap, this setups the browser REPL loop, because that will send back :result
will get there, thanks for the explanation, I added some println 😄
@mfikes ^ I’m 80% sure the single REPL bug is that you get back two :results
before you send a response
OK. This path I'm going down is probably a waste. I'll wrap it up and revisit the queue idea.
single swap!
revisions didn't affect the test I'm doing, so gonna look at the two :results
concept
FWIW here is my revised code that I think might be thread-safe / immune to races
https://gist.github.com/mfikes/0c8cf0630b3c86dea5e7423046745b79 perhaps @richiardiandrea wrote something along the same lines
Yes, something like that, basically using the result of swap!
for doing things
@mfikes so the problem your connection
with that is two threads can enter if no connection or it’s already closed
Well, it just looks like a potential "test" then "set" concurrency issue could arise there. No specific reason to believe that's happening.
I'm gonna do some logging to see if I can catch it in the act of two :results
(or at least better undertstand what hapens right when it locks up)
that's what I am doing 😉
I am fine with both getting to the same result, no rush, for these kind of things we need double/triple validation 😄
Yep, I agree @dnolen, two threads running through my connection
impl would overwrite the single :promised-conn
slot.
I've updated my gist to detect / log if overwrites are occuring (without actually doing anything to address the issue), and I'm not seeing any logging. So, even though that code probably deserves attention, I'm thinking the problem on my 2009 iMac might be two :result
s coming in or somesuch. Looking there now.
I am close to a solution I think, I will need to try Mikes' test
do I need -c
for the test? it basically stays in this forever:
Compiling client js ...
Serving HTTP on localhost port 9000
Listening for browser REPL connect ...
this is also happening in 1.10.126
command was clj -m cljs.main -r
browser opened correctly
actually lemme check the quick start 😄
Which test are you running @richiardiandrea?
I wanted to try this https://gist.github.com/mfikes/765ac8e43d70b743e3bfbd02ff4703a1
Oh, sorry, that gist refers to an alias, but here is what I have for that alias:
{:extra-deps {org.clojure/clojurescript {:mvn/version "1.10.138"}}
:main-opts ["-m" "cljs.main"]}
oh yeah so my command above should work
ok works let me try the gist
I will attach the patch to CLJS-2627
and @mfikes if you can confirm that it works it would be awesome
So far for me, it reproduces on Safari and Safari Technology Preview, but not Chrome or Firefox. And when it fails, there is zero evidence of anything going wrong in our code that I can find. I've added logging prints everywhere. When it fails, Safari just indicates that it lost the connection so we end up with a form that needs to be evaluated but no response.
uhm, don't have Safari here, so that is even more important that you check for me as well, maybe I am just on the wrong path 😄
I think there is definitely ample room for bugs in our code, but for the particular test I'm doing it might be something isolated to Safari. This report, while under completely different circumstances, results in the same error I'm seeing https://stackoverflow.com/questions/33895463/safari-ajax-request-failed-to-load-resource-the-network-connection-was-lost
oh ok so my patch my be a bit off then, I still think it is an improvement though
by the way it seems that a FIFO queue on open connections is also good for reusing the web browser connections like @bhauman was hoping for
If I comment out the Content-Length
header here https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/repl/server.clj#L166 then things work reliably in Safari on my 2009 iMac.
I always thought concurrency was tough...you win! :))
See https://apple.stackexchange.com/a/107868
A theory could be that (on this slower box) we are failing to serve all the bytes indicated by the Content-Length
header and Safari balks.
That would explain why removing the Content-Length
header would prevent Safari from balking, but it this theory doesn't explain why things generally appear to work without this header. (In other words, you would think that other things would fail if, in fact we were failing to serve all of the bytes.)
Maybe safari guesses the length?
Or the actual payload arrives dunno
If the payload arrives safari knows the length I guess
But it is weird because the payload should be either cut or going through
Well, if you set the Content-Length
header, and you deliver fewer bytes (or perhaps close the connection before delivering all the bytes), then Safari will indicate
Failed to load resource: The network connection was lost.
If you leave this header off, then Safari won't indicate this message in that scenario because the scenario is no longer applicable.
So it really is a fail fast mechanisms
@mfikes have you tried my patch out of curiosity?
@richiardiandrea Your patch does not appear to solve the particular issue I'm looking at.
kk cool
should solve the race
thanks for trying out, i will try to scroll back and see if I can reproduce the race then
https://dev.clojure.org/jira/browse/CLJS-2629 captures the issue I'm seeing and mentions the odd Content-Length
"solution"
@mfikes @richiardiandrea I have a fix for 2627 and it’s much simpler 🙂
I would say mine is robust 😄 less simple
also fun to implement 😉
you removed the mechanism 😄
https://github.com/clojure/clojurescript/commit/120a1b84b3e2e9274ebf22c2ae8dc28166e71c0d
even if I set the the thread delay in @mfikes gist to something very very short this doesn’t break
well, it is the same solution, except you store outside state
same robustness 😄
better, true, happy that you have chosen two queues there, I think we need both
the only difference in my patch
is that I filter out closed connections on usage
I actually stumbled upon a case where I had a Socket Closed
exception
@richiardiandrea well give a shot at breaking this and I’ll think about that
but I cannot say it is reproducible
@mfikes https://dev.clojure.org/jira/browse/CLJS-2629 seems like a dupe maybe, try master
happy that my morning hasn't gone to waste 😄
trying Mike's stress test against master, no problems so far
it also looks faster (eye balling)
so I totally approve 👍 good job folks!
so you’re chasing a tail, so accept is not a problem, you’ll always have a connection to use
true that
it’s definitely less likely now, so that’s a good since, but there’s clearly still a missing thing
Ahh, after 19936 requests in the stress test the 2012 Mac appears to have repro’d https://dev.clojure.org/jira/browse/CLJS-2629