Skip to content

Commit 3089d26

Browse files
mainejmainej
committed
Reject promise when client returns an error
1 parent faa9f3a commit 3089d26

File tree

4 files changed

+104
-20
lines changed

4 files changed

+104
-20
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
## Unreleased
44

5-
- Server requests can be treated as promesa promises.
5+
- Avoid blocking client responses behind client requests or notifications.
66
- In certain situations the server will abort its pending sent requests to avoid
77
blocking the client's messages.
8+
- Server requests can be treated as promesa promises.
9+
- Bump promesa to `10.0.594`
810

911
## v1.8.1
1012

‎README.md‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,28 @@ Sending a request is similar, with `lsp4clj.server/send-request`. This method re
9797
response))
9898
```
9999

100-
The request object presents the same interface as `future`. It responds to `future-cancel` (which also sends `$/cancelRequest`), `realized?`, `future?`, `future-done?` and `future-cancelled?`.
100+
The request object presents the same interface as `future`. It responds to `realized?`, `future?`, `future-done?` and `future-cancelled?`.
101101

102-
If the request is cancelled, later invocations of `deref` will return `:lsp4clj.server/cancelled`.
102+
If you call `future-cancel` on the request object, the server will send the client a `$/cancelRequest`. `$/cancelRequest` is sent only once, although `lsp4clj.server/deref-or-cancel` or `future-cancel` can be called multiple times. After a request is cancelled, later invocations of `deref` will return `:lsp4clj.server/cancelled`.
103103

104-
`$/cancelRequest` is sent only once, although `lsp4clj.server/deref-or-cancel` or `future-cancel` can be called multiple times.
104+
Alternatively, you can convert the request to a promesa promise, and handle it using that library's tools:
105+
106+
```clojure
107+
(let [request (lsp4clj.server/send-request server "..." params)]
108+
(-> request
109+
(promesa/promise)
110+
(promesa/then (fn [response] {:result :client-success
111+
:value 1
112+
:resp response}))
113+
(promesa/catch (fn [ex-response] {:result :client-error
114+
:value 10
115+
:resp (ex-data ex-response)}))
116+
(promesa/timeout 10000 {:result :timeout
117+
:value 100})
118+
(promesa/then #(update % :value inc))))
119+
```
120+
121+
In this case `(promesa/cancel! request)` will send `$/cancelRequest`.
105122

106123
### Start and stop a server
107124

‎src/lsp4clj/server.clj‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
`(binding [*out* null-output-stream-writer]
2929
~@body))
3030

31+
(defn- resolve-ex-data [p]
32+
(p/catch p (fn [ex] (or (ex-data ex) (p/rejected ex)))))
33+
3134
(defprotocol IBlockingDerefOrCancel
3235
(deref-or-cancel [this timeout-ms timeout-val]))
3336

@@ -36,10 +39,10 @@
3639
(deref [_] (deref p))
3740
clojure.lang.IBlockingDeref
3841
(deref [_ timeout-ms timeout-val]
39-
(deref p timeout-ms timeout-val))
42+
(deref (resolve-ex-data p) timeout-ms timeout-val))
4043
IBlockingDerefOrCancel
4144
(deref-or-cancel [_ timeout-ms timeout-val]
42-
(let [result (deref p timeout-ms ::timeout)]
45+
(let [result (deref (resolve-ex-data p) timeout-ms ::timeout)]
4346
(if (identical? ::timeout result)
4447
(do (p/cancel! p)
4548
timeout-val)
@@ -346,14 +349,9 @@
346349
(if-let [{:keys [p started] :as req} (get pending-requests id)]
347350
(do
348351
(trace this trace/received-response req resp started now)
349-
;; We resolve the promise whether or not the client completed the
350-
;; request successfully. The language-server is expected to
351-
;; determine whether there was an error with something like:
352-
;; `(let [{:keys [error] :as resp} (deref-or-cancel ,,,)])`
353-
;; It would be somewhat more elegant to reject the promise when the
354-
;; client had errors, so that the language-server could use tools
355-
;; like `p/catch`.
356-
(p/resolve! p (if error resp result)))
352+
(if error
353+
(p/reject! p (ex-info "Received error response" resp))
354+
(p/resolve! p result)))
357355
(trace this trace/received-unmatched-response resp now)))
358356
(catch Throwable e
359357
(log-error-receiving this e resp))))

‎test/lsp4clj/server_test.clj‎

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,20 +385,87 @@
385385
_ (server/start server nil)
386386
req (p/promise (server/send-request server "req" {:body "foo"}))
387387
client-rcvd-msg (h/assert-take output-ch)]
388-
(async/put! input-ch (lsp.responses/response (:id client-rcvd-msg) {:result :processed
389-
:value 1}))
390-
(is (= {:result :processed
391-
:value 2}
388+
(async/put! input-ch (lsp.responses/response (:id client-rcvd-msg) {:result "good"}))
389+
(is (= {:result :client-success
390+
:value 2
391+
:resp {:result "good"}}
392392
(-> req
393-
(p/timeout 1000 {:result :test-timeout
394-
:value 1})
393+
(p/then (fn [resp] {:result :client-success
394+
:value 1
395+
:resp resp}))
396+
(p/catch (fn [error-resp-ex] {:result :client-error
397+
:value 10
398+
:resp (ex-data error-resp-ex)}))
399+
(p/timeout 1000 {:result :timeout
400+
:value 100})
395401
(p/then #(update % :value inc))
396402
(deref))))
397403
(is (p/done? req))
398404
(is (p/resolved? req))
399405
(is (not (p/rejected? req)))
400406
(is (not (p/cancelled? req)))
401407
(server/shutdown server)))
408+
(testing "after timeout"
409+
(let [input-ch (async/chan 3)
410+
output-ch (async/chan 3)
411+
server (server/chan-server {:output-ch output-ch
412+
:input-ch input-ch})
413+
_ (server/start server nil)
414+
req (p/promise (server/send-request server "req" {:body "foo"}))]
415+
(is (= {:result :timeout
416+
:value 101}
417+
(-> req
418+
(p/then (fn [resp] {:result :client-success
419+
:value 1
420+
:resp resp}))
421+
(p/catch (fn [error-resp-ex] {:result :client-error
422+
:value 10
423+
:resp (ex-data error-resp-ex)}))
424+
(p/timeout 300 {:result :timeout
425+
:value 100})
426+
(p/then #(update % :value inc))
427+
(deref))))
428+
(is (not (p/done? req)))
429+
(is (not (p/resolved? req)))
430+
(is (not (p/rejected? req)))
431+
(is (not (p/cancelled? req)))
432+
(server/shutdown server)))
433+
(testing "after client error"
434+
(let [input-ch (async/chan 3)
435+
output-ch (async/chan 3)
436+
server (server/chan-server {:output-ch output-ch
437+
:input-ch input-ch})
438+
_ (server/start server nil)
439+
req (p/promise (server/send-request server "req" {:body "foo"}))
440+
client-rcvd-msg (h/assert-take output-ch)]
441+
(async/put! input-ch
442+
(-> (lsp.responses/response (:id client-rcvd-msg))
443+
(lsp.responses/error {:code 1234
444+
:message "Something bad"
445+
:data {:body "foo"}})))
446+
(is (= {:result :client-error
447+
:value 11
448+
:resp {:jsonrpc "2.0",
449+
:id 1,
450+
:error {:code 1234,
451+
:message "Something bad",
452+
:data {:body "foo"}}}}
453+
(-> req
454+
(p/then (fn [resp] {:result :client-success
455+
:value 1
456+
:resp resp}))
457+
(p/catch (fn [error-resp-ex] {:result :client-error
458+
:value 10
459+
:resp (ex-data error-resp-ex)}))
460+
(p/timeout 1000 {:result :timeout
461+
:value 100})
462+
(p/then #(update % :value inc))
463+
(deref))))
464+
(is (p/done? req))
465+
(is (not (p/resolved? req)))
466+
(is (p/rejected? req))
467+
(is (not (p/cancelled? req)))
468+
(server/shutdown server)))
402469
(testing "after cancellation"
403470
(let [input-ch (async/chan 3)
404471
output-ch (async/chan 3)

0 commit comments

Comments
 (0)