Skip to content

Commit fc09015

Browse files
authored
Add initial set of TODOs for batch request processing (#425)
## Summary TODOs for batch JSONRPC requests feature.
1 parent d191e0d commit fc09015

File tree

6 files changed

+56
-0
lines changed

6 files changed

+56
-0
lines changed

gateway/gateway.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ func (g Gateway) handleHTTPServiceRequest(
128128
return
129129
}
130130

131+
// TODO_TECHDEBT(@adshmh): Build a single protocol context to handle a request.
132+
// - Obtaining a response to the user's request is protocol context's main responsibility.
133+
// - The protocol context can/should:
134+
// - Use fallback endpoints if needed.
135+
// - Launch parallel requests to multiple endpoints if appropriate.
136+
//
131137
// TODO_CHECK_IF_DONE(@adshmh): Enhance the protocol interface used by the gateway to provide explicit error classification.
132138
// Implementation should:
133139
// 1. Differentiate between user errors (e.g., invalid Service ID in request) and system errors (e.g., endpoint timeout)

gateway/http_request_context_handle_request.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ type parallelRequestMetrics struct {
3737
overallStartTime time.Time
3838
}
3939

40+
// TODO_TECHDEBT(@adshmh): Use a SINGLE protocol context to handle a relay request.
41+
// - Launching multiple parallel requests to multiple endpoints is an internal protocol decision.
42+
//
4043
// HandleRelayRequest sends a relay from the perspective of a gateway.
4144
//
4245
// It performs the following steps:
@@ -79,13 +82,23 @@ func (rc *requestContext) handleSingleRelayRequest() error {
7982
return err
8083
}
8184

85+
// TODO_TECHDEBT(@adshmh): Ensure the protocol returns exactly one response per service payload:
86+
// - Define a struct to contain each service payload and its corresponding response.
87+
// - protocol should return this new struct to clarify mapping of service payloads and the corresponding endpoint response.
88+
// - QoS packages should use this new struct to prepare the user response.
89+
// - Remove the individual endpoint response handling from the gateway package.
90+
//
8291
for _, endpointResponse := range endpointResponses {
8392
rc.qosCtx.UpdateWithResponse(endpointResponse.EndpointAddr, endpointResponse.Bytes)
8493
}
8594

8695
return nil
8796
}
8897

98+
// TODO_TECHDEBT(@adshmh): Remove this method:
99+
// Parallel requests are an internal detail of a protocol integration package
100+
// As of PR #388, `protocol/shannon` is the only protocol integration package.
101+
//
89102
// handleParallelRelayRequests orchestrates parallel relay requests and returns the first successful response.
90103
func (rc *requestContext) handleParallelRelayRequests() error {
91104
metrics := &parallelRequestMetrics{

observation/protocol/shannon.pb.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

proto/path/protocol/shannon.proto

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,9 @@ message ShannonEndpointObservation {
309309
// HTTP Response payload size
310310
optional int64 endpoint_backend_service_http_response_payload_size = 15;
311311

312+
// TODO_TECHDEBT(@adshmh): Separate fallback endpoints into a separate message:
313+
// Most of fields above (e.g. session_id) only apply to a Shannon endpoint.
314+
//
312315
// TODO_CONSIDERATION(@adshmh): Consider renaming to is_gateway_owned OR is_off_protocol.
313316
//
314317
// Tracks whether the endpoint is a fallback endpoint.

protocol/shannon/context.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,14 @@ func (rc *requestContext) sendSingleRelay(payload protocol.Payload) (protocol.Re
146146
return relayResponse, err
147147
}
148148

149+
// TODO_TECHDEBT(@adshmh): Single and Multiple payloads should be handled as similarly as possible:
150+
// - This includes using similar execution paths.
151+
//
152+
// TODO_TECHDEBT(@adshmh): Use the same endpoint response processing used in single relay requests:
153+
// - Use the following on every parallel request:
154+
// - handleEndpointSuccess
155+
// - handleEndpointError
156+
//
149157
// handleParallelRelayRequests orchestrates parallel relay requests to a single endpoint.
150158
// Uses concurrent processing while maintaining response order and proper error handling.
151159
func (rc *requestContext) handleParallelRelayRequests(payloads []protocol.Payload) ([]protocol.Response, error) {
@@ -189,6 +197,8 @@ func (rc *requestContext) launchParallelRelays(payloads []protocol.Payload) <-ch
189197
return resultChan
190198
}
191199

200+
// TODO_TECHDEBT(@adshmh): Define/configure limits for the number of parallel requests from a single context.
201+
//
192202
// executeParallelRelay handles a single relay request in a goroutine
193203
func (rc *requestContext) executeParallelRelay(
194204
payload protocol.Payload,
@@ -238,6 +248,9 @@ func (rc *requestContext) waitForAllRelayResponses(
238248
return rc.convertResultsToResponses(results, firstErr)
239249
}
240250

251+
// TODO_TECHDEBT(@adshmh): Handle EVERY error encountered in parallel requests.
252+
// TODO_TECHDEBT(@adshmh): Support multiple endpoints for parallel requests.
253+
//
241254
// convertResultsToResponses converts parallel relay results into an array of protocol responses.
242255
// Maintains the order of responses to match the order of input payloads.
243256
func (rc *requestContext) convertResultsToResponses(results []parallelRelayResult, firstErr error) ([]protocol.Response, error) {

qos/evm/request_validator.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ const (
188188
jsonrpcBatchRequest
189189
)
190190

191+
// TODO_TECHDEBT(@adshmh): JSONRPC handling should be done in the qos/jsonrpc package:
192+
// - Parse a []byte into a JSONRPC request: detect whether it is a single or a batch request.
193+
// - Validate the request: e.g. no duplicate IDs in a batch request.
194+
// - Determine the response format: e.g. response to a single request formatted as a batch must also be formatted as a batch.
195+
// - Support building observations.
196+
//
191197
// TODO_MVP(@adshmh): Add a JSON-RPC request validator to reject invalid/unsupported
192198
// method calls early in request flow.
193199
//
@@ -211,6 +217,8 @@ func parseJSONRPCFromRequestBody(
211217
return nil, err
212218
}
213219

220+
// TODO_TECHDEBT(@adshmh): Move the parsing logic to qos/jsonrpc package.
221+
//
214222
// Step 2: Detect request format (batch vs single)
215223
requestFormat := detectRequestFormat(trimmedBody)
216224

@@ -225,6 +233,9 @@ func parseJSONRPCFromRequestBody(
225233
}
226234
}
227235

236+
// TODO_TECHDEBT(@adshmh): Move this to qos/jsonrpc package.
237+
// Parsing (via encoding/json) should handle this without the need for extra validation logic.
238+
//
228239
// validateRequestBody performs initial validation on the HTTP request body.
229240
// It trims whitespace and ensures the body is not empty.
230241
//
@@ -240,6 +251,10 @@ func validateRequestBody(logger polylog.Logger, requestBody []byte) ([]byte, err
240251
return trimmedBody, nil
241252
}
242253

254+
// TODO_TECHDEBT(@adshmh): Handle all the parsing in qos/jsonrpc package:
255+
// - No need to check for specific characters.
256+
// - encoding/json Unmarshaling can handle different cases: single vs. batch.
257+
//
243258
// detectRequestFormat analyzes the trimmed request body to determine if it represents
244259
// a single JSON-RPC request or a batch request.
245260
//
@@ -263,6 +278,8 @@ func detectRequestFormat(trimmedBody []byte) jsonrpcRequestFormat {
263278
}
264279
}
265280

281+
// TODO_TECHDEBT(@adshmh): move any validation to the qos/jsonrpc package.
282+
//
266283
// parseBatchRequest handles parsing of JSON-RPC batch requests (JSON arrays).
267284
// Performs validation to ensure:
268285
// - The array can be unmarshaled into JSON-RPC request structures

0 commit comments

Comments
 (0)