Avoid intermediate strings in MCP transport serialization#1274
Avoid intermediate strings in MCP transport serialization#1274
Conversation
- StreamServerTransport.SendMessageAsync: Use SerializeToUtf8Bytes instead of Serialize + Encoding.UTF8.GetBytes - StreamClientSessionTransport.SendMessageAsync: Refactor to write UTF-8 bytes directly to the stream instead of going through TextWriter with a string - McpHttpClient (netstandard2.0 path): Use SerializeToUtf8Bytes + ByteArrayContent instead of Serialize + StringContent - ClientOAuthProvider: Use SerializeToUtf8Bytes + ByteArrayContent instead of Serialize + StringContent Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
The refactoring to bypass TextWriter and write directly to the underlying Stream changed the timing of pipe writes, causing the StdioClientTransportTests.CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked test to fail on macOS and Ubuntu CI. Reverting to keep the original TextWriter-based write path while still using the deferred logging overload to avoid unnecessary serialization when trace logging is disabled. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Fixed in 523a0cb. The |
|
@copilot that timing explanation doesn't make sense. Timing issues like that should not result in test failure, and if they do, that's an issue to be fixed. |
…f8Bytes The previous CI failure was a pre-existing flaky test (StdErrCallbackInvoked), not caused by this change. The test's stderr callback mechanism (process.BeginErrorReadLine in StdioClientTransport) is independent of how stdin writes are performed. Changes: - Remove TextWriter-based constructor and StreamWriter wrapping - Store raw Stream for writes, use SerializeToUtf8Bytes + direct stream writes - Keep TextReader wrapping for reads (needed for ReadLineAsync) Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
You're right — the CI failure ( |
Use a static readonly MediaTypeHeaderValue instance shared between McpHttpClient and ClientOAuthProvider instead of creating a new instance on every request. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
|
cc: @ericstj |
Several transport paths serialize JSON to a
string, then re-encode to UTF-8 bytes. This eliminates those intermediate allocations by going directly to UTF-8.StreamServerTransport.SendMessageAsync: ReplaceJsonSerializer.Serialize+Encoding.UTF8.GetByteswithJsonSerializer.SerializeToUtf8Bytesto write directly to the output streamStreamClientSessionTransport.SendMessageAsync: RemoveStreamWriterindirection — useSerializeToUtf8Bytesand write bytes directly to the underlyingStream(same pattern asStreamServerTransport)McpHttpClient.CreatePostBodyContent(netstandard2.0 path): ReplaceJsonSerializer.Serialize+new StringContent(...)withSerializeToUtf8Bytes+ByteArrayContentClientOAuthProvider: ReplaceJsonSerializer.Serialize+new StringContent(...)withSerializeToUtf8Bytes+ByteArrayContentMediaTypeHeaderValue: Add a shared staticMediaTypeHeaderValueinstance forapplication/json; charset=utf-8used by bothMcpHttpClientandClientOAuthProvider, avoiding per-request allocationsBoth
SendMessageAsyncimplementations also now use the deferredLogTransportSendingMessageSensitive(JsonRpcMessage)overload, which only serializes for logging when trace is actually enabled.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.