flow-storm / flow-storm-debugger

A debugger for Clojure and ClojureScript with some unique features.
The Unlicense
708 stars 32 forks source link

Unexpected behavior automatic namespaces reload #184

Closed velios closed 4 months ago

velios commented 4 months ago

Hi. Thx for new great and big 3.17.0 release.

I tried to use the new functionality, but came across unexpected (perhaps because I don’t understand the principles of reload) behavior.

To illustrate, I have prepared a simple demo with pedestal. repo

Steps I follow:

  1. clj -M:dev
  2. (require 'hello)
  3. :dbg(run flowstorm)
  4. (hello/start) ;; <- now our server started on 8890 port
  5. !Difference in this step! If before add instr prefix i even one time make call through curl -i http://127.0.0.1:8890/greet i don't see io.pedestal.interceptor.chain/try-f calls through Flows -> Quich jump
  6. move to flowstorm ui 6.1. Browser tab 6.2. Search for io.pedestal.interceptor.chain and add instr prefix for io.pedestal.interceptor.chain.* 6.3 Apply and see unload/reload log in repl
  7. Make curl -i http://127.0.0.1:8890/greet. If you didn't do step 5 after call you will see fn call if write try-f in Flows -> Quich jump. If you made step 5 before nothing shows when write try-f in Flows -> Quich jump.

Using the same scheme, nothing changes when a namespace pattern is removed from the list in Browse. I think because you make call before remove ns regex.

If the problem is not reproduced or my explanation was not clear, then let me know and I will record a video explanation.

jpmonettas commented 4 months ago

@velios just sent a PR to your repo with some instructions. Can you try them?

velios commented 4 months ago

@jpmonettas Thanks for such a quick response. In fact, with the changes from PR I couldn't get any results at all in flow-storm. I decided that the video could still be more informative, so I’m attaching it.

https://github.com/user-attachments/assets/5188f94e-467f-49a8-8d85-f8e8ae3f3643

jpmonettas commented 4 months ago

@velios after you update the UI (read arrow) what functions do you see captured in the functions list?

jpmonettas commented 4 months ago

https://github.com/user-attachments/assets/192e10dc-3381-4f55-8674-0db920b8a09d

velios commented 4 months ago

@jpmonettas I'm not sure about answer to last qustion. I can upload screenshot from function list tool after last frame on video.

image

But if i restart all stuff with "-Dclojure.storm.instrumentOnlyPrefixes=io.pedestal.interceptor.chain" option in config function list tool looks like this after http-request

image
jpmonettas commented 4 months ago

@velios can you try it from the terminal like on my video?

velios commented 4 months ago

@jpmonettas In the console everything works the same as in your video. Also everything works fine on my work projects if i stick to halt system -> modify instrumented ns -> resume system cycle. So I have a question. Can I enable and disable namespaces that interest me through the interface and then programmatically restart the namespaces. Programmatically so that I can add this logic to my project lifecycle management namespace.

velios commented 4 months ago

Or maybe in the flowstorm interface we need a button for manually updating namespaces not only after hit Apply button. As ability to hit it moment when the system is ready for restart namespaces, for example if it correctly stopped.

jpmonettas commented 4 months ago

So, the halt system -> modify instrumentation -> resume system shouldn't be needed for most cases, only when there are stateful namespaces involved like in these webserver cases. If you instrument/reload namespaces with plain functions you shouldn't need to do anything.

Can I enable and disable namespaces that interest me through the interface and then programmatically restart the namespaces. Programmatically so that I can add this logic to my project lifecycle management namespace.

I'm not sure what you mean by enable/disable or restarting namespaces.

In any case I'll need to go deeper into this issue, because there is something funky going on here.

velios commented 4 months ago

@jpmonettas After your explanation, I no longer think there is any problem here. I am also attaching a video in which I explain what questions I now have when you explained to me the correct chain of actions.

https://github.com/user-attachments/assets/30e3524b-c022-4cdb-8da5-6764710ee007

jpmonettas commented 4 months ago

@velios I think I found the culprit and fixed it. Can you try with this coordinates? com.github.flow-storm/flow-storm-debugger {:git/sha "45081ec744e3d688abf3ee4a9523ece7344857a5"}

The fix has to do with it working when you do (require 'hello) but not working when the ide does a (load-file "...."). With that commit should work no matter how you load the hello namespace.

jpmonettas commented 4 months ago

Yeah we can add a extra option to the right click of the namespaces browser to reload without adding a prefix, I think that would solve that problem, so we can reload on demand, without instrumentation involved.

In the meantime if you forget, you can add a prefix again and delete it immediately whenever you want to reload.

velios commented 4 months ago

@jpmonettas Unfortunately after fix it not work at all, because of cursive internals. After hit Apply thrown exception

Exception in thread "JavaFX Application Thread" java.io.FileNotFoundException: /Users/velios/Library/Application Support/JetBrains/IntelliJIdea2023.3/plugins/clojure-plugin/lib/runtime.jar!/cursive.clj (No such file or directory)
    at java.base/java.io.FileInputStream.open0(Native Method)
    at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
    at java.base/java.io.FileInputStream.<init>(FileInputStream.java:152)
    at clojure.java.io$fn__11661.invokeStatic(io.clj:238)
    at clojure.java.io$fn__11661.invoke(io.clj:235)
    at clojure.java.io$fn__11609$G__11563__11616.invoke(io.clj:692)
    at clojure.java.io$fn__11631.invokeStatic(io.clj:165)
    at clojure.java.io$fn__11631.invoke(io.clj:165)
    at clojure.java.io$fn__11570$G__11559__11577.invoke(io.clj:692)
    at clojure.java.io$reader.invokeStatic(io.clj:102)
    at clojure.java.io$reader.doInvoke(io.clj:86)
    at clojure.lang.RestFn.invoke(RestFn.java:410)
    at flow_storm.ns_reload_utils$read_file_or_resource.invokeStatic(ns_reload_utils.clj:205)
    at flow_storm.ns_reload_utils$read_file_or_resource.invoke(ns_reload_utils.clj:200)
    at flow_storm.ns_reload_utils$reload_all$fn__10676.invoke(ns_reload_utils.clj:307)
    at clojure.core.protocols$iter_reduce.invokeStatic(protocols.clj:49)
    at clojure.core.protocols$fn__8258.invokeStatic(protocols.clj:75)
    at clojure.core.protocols$fn__8258.invoke(protocols.clj:75)
    at clojure.core.protocols$fn__8206$G__8201__8219.invoke(protocols.clj:692)
    at clojure.core$reduce.invokeStatic(core.clj:6887)
    at clojure.core$reduce.invoke(core.clj:6869)
    at flow_storm.ns_reload_utils$reload_all.invokeStatic(ns_reload_utils.clj:306)
    at flow_storm.ns_reload_utils$reload_all.invoke(ns_reload_utils.clj:287)
    at flow_storm.debugger.ui.browser.screen$modify_storm_instrumentation.invokeStatic(screen.clj:188)
    at flow_storm.debugger.ui.browser.screen$modify_storm_instrumentation.invoke(screen.clj:172)
    at flow_storm.debugger.ui.browser.screen$modify_storm_instrumentation.invokeStatic(screen.clj:173)
    at flow_storm.debugger.ui.browser.screen$modify_storm_instrumentation.invoke(screen.clj:172)
    at flow_storm.debugger.ui.browser.screen$create_namespaces_pane$fn__10836$fn__10838$fn__10839.invoke(screen.clj:215)
    at flow_storm.debugger.ui.components$context_menu$fn__7806$event_handler_fn__7808.invoke(components.clj:34)
    at flow_storm.debugger.ui.utils$event_handler_STAR_$reify__7709.handle(utils.clj:61)
    at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
    at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
    at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
    at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
    at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
    at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
    at javafx.event.Event.fireEvent(Event.java:198)
    at javafx.scene.control.MenuItem.fire(MenuItem.java:459)
    at com.sun.javafx.scene.control.ContextMenuContent$MenuItemContainer.doSelect(ContextMenuContent.java:1415)
    at com.sun.javafx.scene.control.ContextMenuContent$MenuItemContainer.lambda$createChildren$12(ContextMenuContent.java:1368)
    at com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247)
    at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
    at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
    at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
    at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
    at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
    at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
    at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
    at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
    at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
    at javafx.event.Event.fireEvent(Event.java:198)
...
velios commented 4 months ago

Yeah we can add a extra option to the right click

@jpmonettas Ability to call reload-ns-fn from like flow-storm.api/reload-ns or another place will be a good option too. Not cool to always do it from UI =) This feature is probably already available, I just don’t know how to call it. Can you tell me the direction?

jpmonettas commented 4 months ago

Oh yes, it is flow-storm.ns-reload-utils/reload-all and accepts a regex. You can call it like :

(flow-storm.ns-reload-utils/reload-all #"io.pedestal.*")

Regarding the exception I'll take a look at it tomorrow, but should be pretty easy to fix.

jpmonettas commented 4 months ago

@velios went ahead and gave it a try. This coordinate should get rid of your exception com.github.flow-storm/flow-storm-debugger {:git/sha "908795d404b60c45f279dafd3d2a7017d0a8c0b5"}

Leaving for today!

velios commented 4 months ago

Oh yes, it is flow-storm.ns-reload-utils/reload-all and accepts a regex

No, what i mean is like. Reload current namespace state in UI. (flow-storm.ns-reload-utils/reload-all )

velios commented 4 months ago

908795d404b60c45f279dafd3d2a7017d0a8c0b5

Still same cursive exception

Leaving for today!

See you next time. Thank you for you time.

jpmonettas commented 4 months ago

@velios can you try com.github.flow-storm/flow-storm-debugger {:git/sha "7b081e1e052659fc28715b447ef049e41d98c91a"}. You shouldn't get an exception with this commit. If for same reason you still get it, can you please paste the full exception again?

velios commented 4 months ago

@jpmonettas with 7b081e1e052659fc28715b447ef049e41d98c91a works fine, no exceptions when hit Apply and reload in IDEA

jpmonettas commented 4 months ago

@velios great, I'll be closing this issue then, please re-open it or create a new one if you hit any other issues with reloading.

velios commented 4 months ago

@jpmonettas Another interesting messages in repl in IDEA

I stop system -> load from Browser io.pedestal.interceptor.chain. -> hit Apply -> all reload -> load from Browser hello. -> hit Apply (messages with failures here) -> start system

The functionality works fine, events are also instrumented. I don’t know what these errors affected, I didn’t notice anything. But I decided to provide information.

Unloading hello
Unloading io.pedestal.http
Unloading io.pedestal.http.ring-middlewares
Unloading io.pedestal.http.impl.servlet-interceptor
Unloading io.pedestal.http.tracing
Unloading io.pedestal.http.route
Unloading io.pedestal.interceptor.chain
Unloading io.pedestal.interceptor.chain.debug
Loading io.pedestal.interceptor.chain.debug
Loading io.pedestal.interceptor.chain
Loading io.pedestal.http.route
Loading io.pedestal.http.tracing
Loading io.pedestal.http.impl.servlet-interceptor
Loading io.pedestal.http.ring-middlewares
Loading io.pedestal.http
Loading hello
Failed to read /Users/velios/Library/Application Support/JetBrains/IntelliJIdea2023.3/plugins/clojure-plugin/lib/runtime.jar!/cursive.clj ignoring...
Failed to read file:/Users/velios/.m2/repository/ring/ring-codec/1.2.0/ring-codec-1.2.0.jar!/ring/util/codec.clj java.lang.RuntimeException: EvalReader not allowed when *read-eval* is false.
Failed to read /Users/velios/Library/Application Support/JetBrains/IntelliJIdea2023.3/plugins/clojure-plugin/lib/runtime.jar!/cursive/repl/runtime/class.clj ignoring...
Failed to read /Users/velios/Library/Application Support/JetBrains/IntelliJIdea2023.3/plugins/clojure-plugin/lib/runtime.jar!/cursive/repl/runtime.clj ignoring...
Unloading hello
Loading hello
velios commented 4 months ago

@jpmonettas I can't reopen this issue =(

jpmonettas commented 4 months ago

I'll have to go deeper into what are those cursive files. The reload system tries to read every loaded file to figure out dependencies between them for the reload process. When reading files it looks like it isn't being able to read some of the cursive injected files, but I don't think this matters because nor your files or your libraries files should depend on them.

I would say ignore them for now unless you see they break something in any way. Since this are all dev time things only, if they don't break your dev experience in anyway I think it is safe to ignore those warnings.