Skip to content

Commit 299c90b

Browse files
authored
feat(extension): migrate task transport from gRPC to JSON-RPC over TCP loopback (PR 2/3) (#1864)
* feat(gradle-server): migrate task transport from gRPC to JSON-RPC over TCP loopback Replace the existing gRPC + Netty HTTP/2 transport for the task server with LSP4J JSON-RPC over a plain TCP loopback socket. This eliminates the mid-frame HTTP/2 race (vscode-gradle issue #1815) at its source by removing the Netty stack from the task-server hot path entirely. What changes on the gradle-server side - New package ransport.jsonrpc exposing six methods on gradle/ segment: getBuild, runBuild, getProjectDependencies, cancelBuild, cancelBuilds, executeCommand. Streaming RPCs (getBuild/runBuild) deliver intermediate progress through gradle/getBuild/reply and gradle/runBuild/reply notifications and complete the JSON-RPC request with the terminal reply. - Protobuf wire schema (proto/gradle.proto) is reused for payload bytes; request/response/notification params carry the base64-encoded proto bytes plus a stream id. This keeps the message layer stable across the migration and avoids a JSON re-encoding of large progress payloads. - GradleServer becomes a thin bootstrap: it connects out to the Node listener on 127.0.0.1:<port>, builds an LSP4J launcher backed by a cached worker pool, and blocks until the socket closes. - Handlers no longer hold gRPC StreamObservers. Streaming handlers take (Request, CompletableFuture<GradleResponse>, GradleClient, long streamId); unary handlers take (Request, CompletableFuture<GradleResponse>). Removed - io.grpc:grpc-protobuf, grpc-stub, grpc-netty (plus grpc-testing) dependencies, the protoc-gen-grpc-java plugin, and the �uild/generated/source/proto/main/grpc source set. - TaskService (gRPC service shim) and ErrorMessageBuilder (only used by the gRPC error path). - The Netty HTTP/2 mid-frame log filter and its tests (GradleServerFilterTest). - Forced etty-bom pin (no longer relevant since Netty is gone from the task-server runtime). Tests - GradleServerTest now exercises handlers directly via CompletableFuture<GradleResponse> and a Mockito-mocked GradleClient, so the test surface verifies Gradle Tooling API interactions without spinning up a gRPC InProcess channel. - One extra --add-opens (java.util.concurrent) is added to the test JVM args so PowerMock can introspect CompletableFuture on JDK 17+. This commit is the gradle-server half of the migration. The TypeScript extension still speaks gRPC and is updated in PR 2; the proto files and the root-build grpcVersion constant are cleaned up in PR 3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(gradle-server): cover JSON-RPC transport with end-to-end roundtrip tests Add JsonRpcTransportTest exercising the new transport package end-to-end over a pair of piped streams. Two Launcher instances are wired together (server side: GradleService stub + GradleClient remote proxy; client side: recording GradleClient + GradleService remote proxy) so the full request/response/notification path is driven without a real socket and without a dependency on the Gradle Tooling API. Coverage: - Base64 protobuf encode/decode roundtrip via JsonRpcCodec. - All six RPC methods (getBuild, unBuild, getProjectDependencies, cancelBuild, cancelBuilds, �xecuteCommand) — payload identity, stream-id propagation, and null eply handling. - Streaming notifications: getBuild/reply carries the right stream id and payload to the client side; unBuild/reply is delivered on the separate unBuild notification channel without leaking into the getBuild channel. - Error mapping across the wire for ResponseErrorException produced by JsonRpcCodec.error(...) — covers NOT_FOUND, CANCELLED and INTERNAL (the last one via a Throwable carrier). - Pinned constants for the four error codes that form part of the wire contract with the TS client, to catch accidental renumbering. The handler-level behaviour (Gradle Tooling API interaction, debug init script, JVM args, etc.) stays covered by GradleServerTest, which talks to handlers directly via CompletableFuture<GradleResponse> rather than through the JSON-RPC dispatch layer — so the two test classes between them cover the wire layer and the business logic without overlapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: update the review comments * fix(gradle-server): address Copilot review comments on PR #1863 - GradleServiceImpl: validate JSON-RPC params/request nullability and map Base64 decode errors to ERROR_UNKNOWN instead of ERROR_INTERNAL so client-side input bugs surface with the correct wire code. - ExecuteCommandHandler: split error replies so unknown commands return ERROR_NOT_FOUND and argument-count violations return ERROR_UNKNOWN; reserve ERROR_INTERNAL for genuine server failures. - GradleServer: extract a workerThreadFactory() that hands out unique `gradle-jsonrpc-worker-N` names via AtomicInteger so concurrent handlers are distinguishable in thread dumps and logs. Adds regression coverage: - JsonRpcTransportTest: 4 new tests for null params, null request, invalid Base64, and null cancelBuilds params. - ExecuteCommandHandlerTest (new): 4 tests covering the new error code mapping plus the happy path. - GradleServerThreadFactoryTest (new): 3 tests covering unique naming, daemon flag, and per-instance counter isolation. * perf: improve sync lock and order code * perf: error code * chore: ignore local-only *.local.md working notes * feat(extension): migrate task transport from gRPC to JSON-RPC over TCP loopback (PR 2/3) Pairs with PR 1 (gradle-server Java side) to complete the cut-over from @grpc/grpc-js to vscode-jsonrpc as the wire transport for task RPCs: - Add `extension/src/transport/jsonrpc/` package: `GradleJsonRpcClient` (typed facade over `MessageConnection` exposing the six `gradle/*` methods plus the two `gradle/*/reply` streaming notifications), `loopbackServer` (binds `127.0.0.1:0` and resolves to a `MessageConnection` on inbound connect), `protoCodec`/`streamId`/`JsonRpcErrors` helpers, `types` wire shapes. - Flip `GradleServer` to listen-then-spawn: the extension now binds the ephemeral port before launching the JVM, then passes it via `--port=N`; the JVM (PR 1) connects back as a TCP client. Adds `GradleServer.awaitTaskConnection()` so the client can pick up the inbound `MessageConnection`. - Rewrite `TaskServerClient` end-to-end against `GradleJsonRpcClient`: the six RPCs now go through JSON-RPC, error-branching uses the new `JsonRpcErrors` codes (`NOT_FOUND`/`CANCELLED`/`UNKNOWN`/`INTERNAL`) via `isNotFound`/`isCancelled`/`isUnknown` helpers, and the gRPC-only `retryOnSpuriousCancel` wrappers are dropped (no HTTP/2 stream race exists under plain TCP). - Update `GradleRunnerTerminal.handleError` to use `isUnknown` instead of `grpc.status.UNKNOWN`. - Remove gRPC plumbing: delete `proto/gradle_grpc_pb.{js,d.ts}`, `client/retryOnSpuriousCancel.ts`; strip `protobuf.plugins.grpc` / `task.plugins.grpc` / `task.plugins.ts { option 'service=...' }` from `extension/build.gradle`; drop `@grpc/grpc-js` and `grpc-tools` from `devDependencies` and add `vscode-jsonrpc` (^6.0.0) to `dependencies`. - `Extension.ts`: drop the now-unused gRPC `clientLogger`. Closes #1815 once PR 1 + PR 2 land together. PR 3 will remove the now-unused `service Gradle` block from `proto/gradle.proto` and the `grpcVersion` entry from the root `build.gradle`. * test(extension): cover GradleJsonRpcClient with end-to-end JSON-RPC roundtrip tests Mirrors PR 1's `JsonRpcTransportTest` on the TS side. Wires two `MessageConnection`s back-to-back over `PassThrough` streams and drives `GradleJsonRpcClient` against a vanilla server-side `MessageConnection`, covering: - Base64 proto codec roundtrip. - `getBuild` streaming dispatch (multiple notifications, terminal null response). - Concurrent streaming calls multiplexed by `streamId` without cross-pollination. - `runBuild` streaming dispatch. - Unary `getProjectDependencies` / `cancelBuild` / `cancelBuilds` / `executeCommand`. - `ResponseError` translation back into `GradleRpcError` for the `NOT_FOUND` / `CANCELLED` / `INTERNAL` codes that callers branch on. Also removes the obsolete `retryOnSpuriousCancel` test now that the helper it covered is gone from the gRPC cut-over. * fix(extension): defer loopback listener until after Java env check Self-review caught that `GradleServer.start()` was binding the loopback listener before the `getGradleServerEnv()` null-check. On the no-Java path the early `return` left the listener bound, the 30 s connect timer armed, and the `awaitTaskConnection()` promise un-consumed (`_onDidStart` never fires when env is missing, so `TaskServerClient.connectToServer` is never reached). The eventual rejection surfaced as an `UnhandledPromiseRejection`. Move the listener creation below the env check so the no-Java path leaves no bound socket or pending promise behind. * chore(extension): restore dedicated "jsonrpc" output channel for transport diagnostics Forward vscode-jsonrpc protocol logs into a project Logger channel so transport-level errors/warnings are visible in the existing Gradle output, matching the prior Logger("grpc") behaviour without coupling the transport package to the project Logger class. * test(extension): cover loopback listener port + jsonrpc Logger forwarding Two unit tests for createLoopbackListener: - Asserts an ephemeral 127.0.0.1 port is bound and the connection promise resolves on the first inbound socket. - Asserts a user-supplied vscode-jsonrpc Logger receives protocol-level diagnostics by sending a framed payload that is neither request, response, nor notification and confirming the spy's �rror callback was invoked. * fix(extension): reject loopback connection promise on dispose before accept If createLoopbackListener().dispose() runs before any inbound JVM socket is accepted (e.g. GradleServer's exit handler tearing the listener down after a failed JVM spawn), the connection promise was left unsettled, making TaskServerClient.connectToServer() and other awaiters hang forever. Capture the reject callback and invoke it from dispose() when no socket has been accepted, with a regression test. * fix: process terminal streaming reply in get/runBuild The Java handlers send the terminal GetBuildReply (containing GET_BUILD_RESULT) and terminal RunBuildReply (containing CANCELLED or RUN_BUILD_RESULT) as the JSON-RPC response body, not as a stream notification. The TS side was discarding the response and only processing stream notifications, so getBuild always returned undefined and the task list never populated. Extract the reply handler into a local function and invoke it on the terminal reply returned by getBuild()/runBuild() before returning. * tests: cover terminal streaming reply payload Regression coverage for the fix in 6f8f396: the terminal GetBuildReply / RunBuildReply (carrying GET_BUILD_RESULT or RUN_BUILD_RESULT) arrives on the JSON-RPC response body, not as a stream notification. The new cases assert that getBuild()/runBuild() return the decoded terminal reply so callers can read the build result.
1 parent e39cf04 commit 299c90b

18 files changed

Lines changed: 1102 additions & 967 deletions

extension/build.gradle

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,6 @@ sourceSets {
2121

2222
protobuf {
2323
plugins {
24-
grpc {
25-
path = file(
26-
'./node_modules/.bin/grpc_tools_node_protoc_plugin' + (isWindows ? '.cmd' : '')
27-
)
28-
}
2924
ts {
3025
path = file(
3126
'./node_modules/.bin/protoc-gen-ts' + (isWindows ? '.cmd' : '')
@@ -40,13 +35,7 @@ protobuf {
4035
generateProto.finalizedBy copyProtoJs, copyProtoTs
4136
all().each { task ->
4237
task.plugins {
43-
grpc {
44-
outputSubDir = 'js'
45-
option 'grpc_js'
46-
}
47-
ts {
48-
option 'service=grpc-node,mode=grpc-js'
49-
}
38+
ts {}
5039
}
5140
task.builtins {
5241
remove java

0 commit comments

Comments
 (0)