Skip to content

Commit 1aa8cf2

Browse files
Copilotstephentoub
andcommitted
Address code review feedback: restructure control flow and rename variables
- Rename @params to parameters to avoid needing @ escape character - Move hasJsonRpc declaration into main variable declaration block - Update error message for missing jsonrpc from "Invalid or missing" to "Missing" - Restructure method/id checks to use nested if statements for clarity - Restructure response checks (result/error) to use nested if statements - All 16 JsonRpcMessageConverterTests pass Co-authored-by: Scooletz <scooletz@users.noreply.github.com> Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent e9f325d commit 1aa8cf2

1 file changed

Lines changed: 44 additions & 44 deletions

File tree

src/ModelContextProtocol.Core/Protocol/JsonRpcMessage.cs

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,11 @@ public sealed class Converter : JsonConverter<JsonRpcMessage>
8282
throw new JsonException("Expected StartObject token");
8383
}
8484

85-
// Track whether we've seen the required jsonrpc property
86-
bool hasJsonRpc = false;
87-
8885
// Local variables for parsed message data
86+
bool hasJsonRpc = false;
8987
RequestId id = default;
9088
string? method = null;
91-
JsonNode? @params = null;
89+
JsonNode? parameters = null;
9290
JsonRpcErrorDetail? error = null;
9391
JsonNode? result = null;
9492
bool hasResult = false;
@@ -129,7 +127,7 @@ public sealed class Converter : JsonConverter<JsonRpcMessage>
129127
break;
130128

131129
case "params":
132-
@params = JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<JsonNode>());
130+
parameters = JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<JsonNode>());
133131
break;
134132

135133
case "error":
@@ -151,58 +149,60 @@ public sealed class Converter : JsonConverter<JsonRpcMessage>
151149
// All JSON-RPC messages must have a jsonrpc property with value "2.0"
152150
if (!hasJsonRpc)
153151
{
154-
throw new JsonException("Invalid or missing jsonrpc version");
152+
throw new JsonException("Missing jsonrpc version");
155153
}
156154

157155
// Determine message type based on presence of id and method properties
158-
// Messages with both method and id are requests
159-
if (method is not null && id.Id is not null)
160-
{
161-
return new JsonRpcRequest
162-
{
163-
JsonRpc = JsonRpcVersion,
164-
Id = id,
165-
Method = method,
166-
Params = @params
167-
};
168-
}
169-
170-
// Messages with a method but no id are notifications
171156
if (method is not null)
172157
{
173-
return new JsonRpcNotification
158+
if (id.Id is not null)
159+
{
160+
// Messages with both method and id are requests
161+
return new JsonRpcRequest
162+
{
163+
JsonRpc = JsonRpcVersion,
164+
Id = id,
165+
Method = method,
166+
Params = parameters
167+
};
168+
}
169+
else
174170
{
175-
JsonRpc = JsonRpcVersion,
176-
Method = method,
177-
Params = @params
178-
};
171+
// Messages with a method but no id are notifications
172+
return new JsonRpcNotification
173+
{
174+
JsonRpc = JsonRpcVersion,
175+
Method = method,
176+
Params = parameters
177+
};
178+
}
179179
}
180180

181-
// Messages with a result and id are success responses
182-
if (hasResult && id.Id is not null)
181+
if (id.Id is not null)
183182
{
184-
return new JsonRpcResponse
183+
if (hasResult)
185184
{
186-
JsonRpc = JsonRpcVersion,
187-
Id = id,
188-
Result = result
189-
};
190-
}
185+
// Messages with a result and id are success responses
186+
return new JsonRpcResponse
187+
{
188+
JsonRpc = JsonRpcVersion,
189+
Id = id,
190+
Result = result
191+
};
192+
}
191193

192-
// Messages with an error and id are error responses
193-
if (error is not null && id.Id is not null)
194-
{
195-
return new JsonRpcError
194+
if (error is not null)
196195
{
197-
JsonRpc = JsonRpcVersion,
198-
Id = id,
199-
Error = error
200-
};
201-
}
196+
// Messages with an error and id are error responses
197+
return new JsonRpcError
198+
{
199+
JsonRpc = JsonRpcVersion,
200+
Id = id,
201+
Error = error
202+
};
203+
}
202204

203-
// Error: Messages with an id but no method, error, or result are invalid
204-
if (id.Id is not null)
205-
{
205+
// Error: Messages with an id but no method, error, or result are invalid
206206
throw new JsonException("Response must have either result or error");
207207
}
208208

0 commit comments

Comments
 (0)