Skip to content

Commit 095c3b0

Browse files
attempt to fix issue where cancellation didn't actually do anything useful
1 parent 811d1cb commit 095c3b0

File tree

2 files changed

+80
-18
lines changed

2 files changed

+80
-18
lines changed

src/JsonRpc/InputHandler.cs

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Threading.Tasks;
66
using Newtonsoft.Json.Linq;
77
using Microsoft.Extensions.Logging;
8-
using Newtonsoft.Json;
98
using OmniSharp.Extensions.JsonRpc.Server;
109
using OmniSharp.Extensions.JsonRpc.Server.Messages;
1110

@@ -72,7 +71,8 @@ private void ProcessInputStream()
7271
// content is encoded in UTF-8
7372
while (true)
7473
{
75-
try {
74+
try
75+
{
7676
if (_inputThread == null) return;
7777

7878
var buffer = new byte[300];
@@ -182,8 +182,7 @@ private void HandleRequest(string request)
182182
_scheduler.Add(
183183
type,
184184
item.Request.Method,
185-
async () =>
186-
{
185+
async () => {
187186
try
188187
{
189188
var result = await _requestRouter.RouteRequest(descriptor, item.Request, CancellationToken.None);
@@ -202,26 +201,24 @@ private void HandleRequest(string request)
202201

203202
if (item.IsNotification)
204203
{
204+
205205
var descriptor = _requestRouter.GetDescriptor(item.Notification);
206206
if (descriptor is null) continue;
207+
208+
// We need to special case cancellation so that we can cancel any request that is currently in flight.
209+
if (descriptor.Method == JsonRpcNames.CancelRequest)
210+
{
211+
var cancelParams = item.Notification.Params?.ToObject<CancelParams>();
212+
if (cancelParams == null) { continue; }
213+
_requestRouter.CancelRequest(cancelParams.Id);
214+
continue;
215+
}
216+
207217
var type = _requestProcessIdentifier.Identify(descriptor);
208218
_scheduler.Add(
209219
type,
210220
item.Notification.Method,
211-
async () =>
212-
{
213-
try
214-
{
215-
await _requestRouter.RouteNotification(descriptor, item.Notification, CancellationToken.None);
216-
}
217-
catch (Exception e)
218-
{
219-
_logger.LogCritical(Events.UnhandledNotification, e, "Unhandled exception executing notification {Method}", item.Notification.Method);
220-
// TODO: Should we rethrow or swallow?
221-
// If an exception happens... the whole system could be in a bad state, hence this throwing currently.
222-
throw;
223-
}
224-
}
221+
DoNotification(descriptor, item.Notification)
225222
);
226223
}
227224

@@ -231,6 +228,23 @@ private void HandleRequest(string request)
231228
_outputHandler.Send(item.Error);
232229
}
233230
}
231+
232+
Func<Task> DoNotification(IHandlerDescriptor descriptor, Notification notification)
233+
{
234+
return async () => {
235+
try
236+
{
237+
await _requestRouter.RouteNotification(descriptor, notification, CancellationToken.None);
238+
}
239+
catch (Exception e)
240+
{
241+
_logger.LogCritical(Events.UnhandledNotification, e, "Unhandled exception executing notification {Method}", notification.Method);
242+
// TODO: Should we rethrow or swallow?
243+
// If an exception happens... the whole system could be in a bad state, hence this throwing currently.
244+
throw;
245+
}
246+
};
247+
}
234248
}
235249

236250

test/JsonRpc.Tests/InputHandlerTests.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading.Tasks;
88
using FluentAssertions;
99
using Microsoft.Extensions.Logging;
10+
using Newtonsoft.Json;
1011
using Newtonsoft.Json.Linq;
1112
using NSubstitute;
1213
using OmniSharp.Extensions.JsonRpc;
@@ -262,5 +263,52 @@ public void ShouldHandleResponse()
262263
tcs.Task.Result.ToString().Should().Be("{}");
263264
}
264265
}
266+
267+
[Fact]
268+
public async Task ShouldCancelRequest()
269+
{
270+
var inputStream = new MemoryStream(Encoding.ASCII.GetBytes("Content-Length: 2\r\n\r\n{}"));
271+
var outputHandler = Substitute.For<IOutputHandler>();
272+
var reciever = Substitute.For<IReciever>();
273+
var incomingRequestRouter = Substitute.For<IRequestRouter<IHandlerDescriptor>>();
274+
var requestDescription = Substitute.For<IHandlerDescriptor>();
275+
requestDescription.Method.Returns("abc");
276+
var cancelDescription = Substitute.For<IHandlerDescriptor>();
277+
cancelDescription.Method.Returns(JsonRpcNames.CancelRequest);
278+
279+
var req = new Request(1, "abc", null);
280+
var cancel = new Notification(JsonRpcNames.CancelRequest, "{ \"id\": 1 }");
281+
reciever.IsValid(Arg.Any<JToken>()).Returns(true);
282+
reciever.GetRequests(Arg.Any<JToken>())
283+
.Returns(c => (new Renor[] { req, cancel }, false));
284+
285+
incomingRequestRouter.When(z => z.CancelRequest(Arg.Any<object>()));
286+
incomingRequestRouter.GetDescriptor(cancel).Returns(cancelDescription);
287+
incomingRequestRouter.GetDescriptor(req).Returns(requestDescription);
288+
289+
incomingRequestRouter.RouteRequest(requestDescription, req, CancellationToken.None)
290+
.Returns(new Response(1, req));
291+
292+
incomingRequestRouter.RouteNotification(cancelDescription, cancel, CancellationToken.None)
293+
.Returns(Task.CompletedTask);
294+
295+
using (NewHandler(
296+
inputStream,
297+
outputHandler,
298+
reciever,
299+
Substitute.For<IRequestProcessIdentifier>(),
300+
incomingRequestRouter,
301+
Substitute.For<IResponseRouter>(),
302+
cts => {
303+
outputHandler.When(x => x.Send(Arg.Any<object>()))
304+
.Do(x => {
305+
cts.Cancel();
306+
});
307+
}))
308+
{
309+
await incomingRequestRouter.Received().RouteNotification(cancelDescription, cancel, CancellationToken.None);
310+
incomingRequestRouter.Received().CancelRequest(1L);
311+
}
312+
}
265313
}
266314
}

0 commit comments

Comments
 (0)