RockteMQ-AI commented on code in PR #1375:
URL: https://github.com/apache/rocketmq-clients/pull/1375#discussion_r4012297443
##########
nodejs/src/client/BaseClient.ts:
##########
@@ -512,23 +575,54 @@ export abstract class BaseClient {
const obj = command.toObject();
this.logger.warn('Ignore verify message command from remote, which is not
expected, clientId=%s, command=%j',
this.clientId, obj);
+ // Respond with VerifyMessageResult carrying the same nonce, mirroring the
Java
+ // client (BaseClient#onVerifyMessageCommand), instead of echoing the
command.
const telemetryCommand = new TelemetryCommand();
telemetryCommand.setStatus(new Status().setCode(Code.NOT_IMPLEMENTED));
- telemetryCommand.setVerifyMessageCommand(new
VerifyMessageCommand().setNonce(obj.nonce));
+ telemetryCommand.setVerifyMessageResult(new
VerifyMessageResult().setNonce(obj.nonce));
this.telemetry(endpoints, telemetryCommand);
}
onPrintThreadStackTraceCommand(endpoints: Endpoints, command:
PrintThreadStackTraceCommand) {
const obj = command.toObject();
- this.logger.warn('Ignore orphaned transaction recovery command from
remote, which is not expected, clientId=%s, command=%j',
+ this.logger.info('Received print thread stack trace command from remote,
clientId=%s, command=%j',
this.clientId, obj);
- const nonce = obj.nonce;
const telemetryCommand = new TelemetryCommand();
- telemetryCommand.setThreadStackTrace(new
ThreadStackTrace().setThreadStackTrace('mock stack').setNonce(nonce));
+ telemetryCommand.setThreadStackTrace(new ThreadStackTrace()
+ .setThreadStackTrace(this.#buildProcessDiagnostics())
+ .setNonce(obj.nonce));
telemetryCommand.setStatus(new Status().setCode(Code.OK));
this.telemetry(endpoints, telemetryCommand);
}
+ /**
+ * Build Node.js process diagnostics in place of Java-style thread stack
traces.
+ * Java sends per-thread stacks via ThreadMXBean; Node.js is single-threaded
per
+ * process, so we expose the closest equivalent runtime snapshot.
Review Comment:
`_getActiveHandles()` is an undocumented Node.js internal API. The try/catch
fallback is good defensive coding. Since this is best-effort diagnostics (not
on a critical path), this is acceptable — just be aware it could break silently
in future Node.js versions without notice.
##########
nodejs/src/route/Endpoints.ts:
##########
@@ -52,8 +52,27 @@ export class Endpoints {
this.facade = this.addressesList.map(addr =>
`${addr.host}:${addr.port}`).join(',');
}
+ /**
+ * gRPC target with resolver scheme prefix, mirroring the Java client:
+ * - IPv4 addresses: ipv4:127.0.0.1:10911,127.0.0.2:10912
+ * - IPv6 addresses: ipv6:[::1]:10911,[fe80::1]:10912 (brackets required by
grpc-js)
+ * - Domain names: dns:example.com:8080,example.org:8081
+ */
getGrpcTarget() {
- return this.facade;
+ const targets = this.addressesList.map(addr => {
+ const host = this.scheme === AddressScheme.IPV6 ? `[${addr.host}]` :
addr.host;
+ return `${host}:${addr.port}`;
+ }).join(',');
+ switch (this.scheme) {
+ case AddressScheme.IPV4:
+ return `ipv4:${targets}`;
Review Comment:
Good fix — adding the resolver scheme prefix (`ipv4:` / `ipv6:` / `dns:`)
ensures grpc-js correctly handles multi-address and bare-IPv6 targets. This
aligns with the Java client's `Endpoints.grpcTarget()` behavior. One thing to
verify: ensure the RocketMQ Proxy's gRPC server accepts targets with these
scheme prefixes. Standard grpc-js resolvers should handle them transparently,
but worth confirming in integration tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
