refresh ops shouldn't have a timeout

[vemv]

nrepl-send-sync-request: Sync nREPL request timed out (op refresh-clear) after 10 secs

Messages such as these don't make much sense to me. Code reloading can take arbitrarily long.

Timing out can invite the user to try again while code reloading is already in progress (thankfully we have locking in place, but still leads to unnecessary queueing)

I'd say that for these ops in particular, we could have a timeout of about 3 minutes?

[bbatsov]

Very few projects would need a bigger timeout and the users can easily change it. I’m not sure there’s much we can improve here.

On Thu, May 2, 2024, at 7:00 PM, vemv wrote: `nrepl-send-sync-request: Sync nREPL request timed out (op refresh-clear) after 10 secs

`

…

Messages such as these don't make much sense to me. Code reloading can take arbitrarily long. Timing out can invite the user to try again while code reloading is already in progress (thankfully we have `locking` in place, but still leads to unnecessary queueing) I'd say that for these ops in particular, we could have a timeout of about 3 minutes? — Reply to this email directly, view it on GitHub <#3652>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZLSQDMQVGCYYQHV5PV4LZAJWK7AVCNFSM6AAAAABHEC2S4SVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3TMMBSHEZTSMY>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[vemv]

I see things differently - many commercial-sized projects can easily need 30s+ for a full-blown refresh.

Also, users don't necessarily are familar with (or even will pay much attention to) "nrepl timeouts".

Test suite running is similarly unbounded?

[vemv]

A touch more of tech analysis:

• refreshing is generally async
• however, refresh-clear is sync
• both grab a lock, cider-nrepl side
• so, while refresh-clear itself is fast (it doesn't do code reloading per se), it first has to grab a lock, which may be held by an in-progress code reloading (triggered async)

A very cool alternative approach would be to remove the locking from https://github.com/clojure-emacs/cider-nrepl/blob/79611e47615bc00ea00fe9c265da896976324ae0/src/cider/nrepl/middleware/refresh.clj#L110 , refactoring it to merely enqueue a clearing. So next time refresh-reply is hit, it checks the queue, and performs a clearing if due.