From beaf47008e5405f2831ddb8f75c6a598b39aed90 Mon Sep 17 00:00:00 2001 From: Mayuki Sawatari Date: Thu, 26 Dec 2024 16:07:49 +0900 Subject: [PATCH] Handling when calling a non-implemented hub method --- .../Diagnostics/MagicOnionServerLog.cs | 3 ++ .../Internal/StreamingHubPayloadBuilder.cs | 29 ++++++++++++++ src/MagicOnion.Server/Hubs/StreamingHub.cs | 4 +- .../Hubs/StreamingHubContext.cs | 32 +++------------ .../StreamingHubTest.UnknownMethodId.cs | 39 +++++++++++++++++++ .../StreamingHubTest.cs | 2 +- 6 files changed, 80 insertions(+), 29 deletions(-) create mode 100644 src/MagicOnion.Server/Hubs/Internal/StreamingHubPayloadBuilder.cs create mode 100644 tests/MagicOnion.Integration.Tests/StreamingHubTest.UnknownMethodId.cs diff --git a/src/MagicOnion.Server/Diagnostics/MagicOnionServerLog.cs b/src/MagicOnion.Server/Diagnostics/MagicOnionServerLog.cs index 29e4d05c0..df14c80b1 100644 --- a/src/MagicOnion.Server/Diagnostics/MagicOnionServerLog.cs +++ b/src/MagicOnion.Server/Diagnostics/MagicOnionServerLog.cs @@ -88,6 +88,9 @@ static string MethodTypeToString(MethodType type) => [LoggerMessage(EventId = 16, Level = LogLevel.Trace, EventName = nameof(ServiceMethodNotDiscovered), Message = "Could not found gRPC and StreamingHub methods for '{serviceName}'")] public static partial void ServiceMethodNotDiscovered(ILogger logger, string serviceName); + [LoggerMessage(EventId = 17, Level = LogLevel.Information, EventName = nameof(HubMethodNotFound), Message = "StreamingHub method '{methodId}' was not found in '{hubName}'.")] + public static partial void HubMethodNotFound(ILogger logger, string hubName, int methodId); + [LoggerMessage(EventId = 90, Level = LogLevel.Error, EventName = nameof(ErrorOnServiceMethod), Message = "A service handler throws an exception occurred in {method}")] public static partial void ErrorOnServiceMethod(ILogger logger, Exception ex, string method); diff --git a/src/MagicOnion.Server/Hubs/Internal/StreamingHubPayloadBuilder.cs b/src/MagicOnion.Server/Hubs/Internal/StreamingHubPayloadBuilder.cs new file mode 100644 index 000000000..70da968cd --- /dev/null +++ b/src/MagicOnion.Server/Hubs/Internal/StreamingHubPayloadBuilder.cs @@ -0,0 +1,29 @@ +using MagicOnion.Internal; +using MagicOnion.Internal.Buffers; +using MagicOnion.Serialization; + +namespace MagicOnion.Server.Hubs.Internal; + +internal static class StreamingHubPayloadBuilder +{ + public static StreamingHubPayload Build(int methodId, int messageId) + { + using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); + StreamingHubMessageWriter.WriteResponseMessage(buffer, methodId, messageId); + return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); + } + + public static StreamingHubPayload Build(int methodId, int messageId, T v, IMagicOnionSerializer messageSerializer) + { + using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); + StreamingHubMessageWriter.WriteResponseMessage(buffer, methodId, messageId, v, messageSerializer); + return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); + } + + public static StreamingHubPayload BuildError(int messageId, int statusCode, string detail, Exception? ex, bool isReturnExceptionStackTraceInErrorDetail) + { + using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); + StreamingHubMessageWriter.WriteResponseMessageForError(buffer, messageId, statusCode, detail, ex, isReturnExceptionStackTraceInErrorDetail); + return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); + } +} diff --git a/src/MagicOnion.Server/Hubs/StreamingHub.cs b/src/MagicOnion.Server/Hubs/StreamingHub.cs index e36f29eff..b3e911605 100644 --- a/src/MagicOnion.Server/Hubs/StreamingHub.cs +++ b/src/MagicOnion.Server/Hubs/StreamingHub.cs @@ -312,7 +312,9 @@ async ValueTask ProcessRequestAsync(UniqueHashDictionary ha } else { - throw new InvalidOperationException("Handler not found in received methodId, methodId:" + methodId); + MagicOnionServerLog.HubMethodNotFound(Context.Logger, Context.ServiceName, methodId); + var payload = StreamingHubPayloadBuilder.BuildError(messageId, (int)StatusCode.Unimplemented, $"StreamingHub method '{methodId}' is not found in StreamingHub.", null, isReturnExceptionStackTraceInErrorDetail); + StreamingServiceContext.QueueResponseStreamWrite(payload); } } diff --git a/src/MagicOnion.Server/Hubs/StreamingHubContext.cs b/src/MagicOnion.Server/Hubs/StreamingHubContext.cs index d384c6439..3ee33e98e 100644 --- a/src/MagicOnion.Server/Hubs/StreamingHubContext.cs +++ b/src/MagicOnion.Server/Hubs/StreamingHubContext.cs @@ -1,4 +1,3 @@ -using MagicOnion.Internal.Buffers; using MessagePack; using System.Collections.Concurrent; using MagicOnion.Internal; @@ -102,7 +101,7 @@ internal ValueTask WriteResponseMessageNil(ValueTask value) ResponseType = typeof(Nil); if (value.IsCompletedSuccessfully) { - WriteMessageCore(BuildMessage()); + WriteMessageCore(StreamingHubPayloadBuilder.Build(MethodId, MessageId)); return default; } return Await(this, value); @@ -110,7 +109,7 @@ internal ValueTask WriteResponseMessageNil(ValueTask value) static async ValueTask Await(StreamingHubContext ctx, ValueTask value) { await value.ConfigureAwait(false); - ctx.WriteMessageCore(ctx.BuildMessage()); + ctx.WriteMessageCore(StreamingHubPayloadBuilder.Build(ctx.MethodId, ctx.MessageId)); } } @@ -124,7 +123,7 @@ internal ValueTask WriteResponseMessage(ValueTask value) ResponseType = typeof(T); if (value.IsCompletedSuccessfully) { - WriteMessageCore(BuildMessage(value.Result)); + WriteMessageCore(StreamingHubPayloadBuilder.Build(MethodId, MessageId, value.Result, ServiceContext.MessageSerializer)); return default; } return Await(this, value); @@ -132,13 +131,13 @@ internal ValueTask WriteResponseMessage(ValueTask value) static async ValueTask Await(StreamingHubContext ctx, ValueTask value) { var vv = await value.ConfigureAwait(false); - ctx.WriteMessageCore(ctx.BuildMessage(vv)); + ctx.WriteMessageCore(StreamingHubPayloadBuilder.Build(ctx.MethodId, ctx.MessageId, vv, ctx.ServiceContext.MessageSerializer)); } } internal ValueTask WriteErrorMessage(int statusCode, string detail, Exception? ex, bool isReturnExceptionStackTraceInErrorDetail) { - WriteMessageCore(BuildMessageForError(statusCode, detail, ex, isReturnExceptionStackTraceInErrorDetail)); + WriteMessageCore(StreamingHubPayloadBuilder.BuildError(MessageId, statusCode, detail, ex, isReturnExceptionStackTraceInErrorDetail)); return default; } @@ -147,25 +146,4 @@ void WriteMessageCore(StreamingHubPayload payload) ResponseSize = payload.Length; // NOTE: We cannot use the payload after QueueResponseStreamWrite. streamingServiceContext.QueueResponseStreamWrite(payload); } - - StreamingHubPayload BuildMessage() - { - using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); - StreamingHubMessageWriter.WriteResponseMessage(buffer, MethodId, MessageId); - return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); - } - - StreamingHubPayload BuildMessage(T v) - { - using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); - StreamingHubMessageWriter.WriteResponseMessage(buffer, MethodId, MessageId, v, streamingServiceContext.MessageSerializer); - return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); - } - - StreamingHubPayload BuildMessageForError(int statusCode, string detail, Exception? ex, bool isReturnExceptionStackTraceInErrorDetail) - { - using var buffer = ArrayPoolBufferWriter.RentThreadStaticWriter(); - StreamingHubMessageWriter.WriteResponseMessageForError(buffer, MessageId, statusCode, detail, ex, isReturnExceptionStackTraceInErrorDetail); - return StreamingHubPayloadPool.Shared.RentOrCreate(buffer.WrittenSpan); - } } diff --git a/tests/MagicOnion.Integration.Tests/StreamingHubTest.UnknownMethodId.cs b/tests/MagicOnion.Integration.Tests/StreamingHubTest.UnknownMethodId.cs new file mode 100644 index 000000000..fe4aa638b --- /dev/null +++ b/tests/MagicOnion.Integration.Tests/StreamingHubTest.UnknownMethodId.cs @@ -0,0 +1,39 @@ +using Grpc.Net.Client; +using MagicOnion.Server.Hubs; + +namespace MagicOnion.Integration.Tests +{ + public partial class StreamingHubTest + { + [Theory] + [MemberData(nameof(EnumerateStreamingHubClientFactory))] + public async Task UnknownMethodId(TestStreamingHubClientFactory clientFactory) + { + // Arrange + var httpClient = factory.CreateDefaultClient(); + var channel = GrpcChannel.ForAddress("http://localhost", new GrpcChannelOptions() { HttpClient = httpClient }); + + var receiver = Substitute.For(); + var client = await clientFactory.CreateAndConnectAsync(channel, receiver); + + // Act + var ex = await Record.ExceptionAsync(async () => await client.CustomMethodId().WaitAsync(TimeSpan.FromSeconds(1))); + + // Assert + ex.Should().NotBeNull(); + var rpcException = ex.Should().BeOfType().Subject; + rpcException.Status.StatusCode.Should().Be(StatusCode.Unimplemented); + factory.Logs.Should().Contain(x => x.Contains("HubMethodNotFound\tStreamingHub method '-1'")); + } + } + +} + +namespace MagicOnion.Integration.Tests.UnknownMethodIdTest +{ + public interface IStreamingHubTestHub : IStreamingHub + { + [MethodId(-1)] + Task CustomMethodId(); + } +} diff --git a/tests/MagicOnion.Integration.Tests/StreamingHubTest.cs b/tests/MagicOnion.Integration.Tests/StreamingHubTest.cs index 29906bbf1..b73bd6f63 100644 --- a/tests/MagicOnion.Integration.Tests/StreamingHubTest.cs +++ b/tests/MagicOnion.Integration.Tests/StreamingHubTest.cs @@ -6,7 +6,7 @@ namespace MagicOnion.Integration.Tests; -public class StreamingHubTest : IClassFixture> +public partial class StreamingHubTest : IClassFixture> { readonly MagicOnionApplicationFactory factory;