Trace logs should clearly indicate an error has happened

[rcjsuen]

[Trace - 10:09:25 AM] Received request 'client/registerCapability - (1)'.
Params: [
    {
        "id": "abc",
        "method": "textDocument/formatting",
        "registerOptions": {
            "documentSelector": [
                {
                    "language": "helloworld"
                }
            ]
        }
    }
]


[Trace - 10:16:18 AM] Sending response 'client/registerCapability - (1)'. Processing request took 413323ms

The trace message makes no indication that an error was sent back and I could not figure out why the client/registerCapability was not even being hit. Since the messages were getting logged I knew they were not totally "lost" so I added breakpoints to the vscode-jsonrpc code and realized that VS Code did not like how I had formatted my client/registerCapability request.

vscode-languageserver-node/jsonrpc/src/common/connection.ts

Lines 868 to 872 in dadd73f

	} else if (Array.isArray(requestMessage.params)) {
	if (type !== undefined && type.parameterStructures === ParameterStructures.byName) {
	return replyError(new ResponseError<void>(ErrorCodes.InvalidParams, `Request ${requestMessage.method} defines parameters by name but received parameters by position`), requestMessage.method, startTime);
	}
	handlerResult = requestHandler(...requestMessage.params, cancellationSource.token);

We can then later clearly see that message.error is present and bad but the trace message in the console does not include the -32602 error.

vscode-languageserver-node/jsonrpc/src/common/connection.ts

Lines 1076 to 1098 in dadd73f

	function traceSendingResponse(message: ResponseMessage, method: string, startTime: number): void {
	if (trace === Trace.Off || !tracer) {
	return;
	}
	if (traceFormat === TraceFormat.Text) {
	let data: string | undefined = undefined;
	if (trace === Trace.Verbose || trace === Trace.Compact) {
	if (message.error && message.error.data) {
	data = `Error data: ${stringifyTrace(message.error.data)}\n\n`;
	} else {
	if (message.result) {
	data = `Result: ${stringifyTrace(message.result)}\n\n`;
	} else if (message.error === undefined) {
	data = 'No result returned.\n\n';
	}
	}
	}
	tracer.log(`Sending response '${method} - (${message.id})'. Processing request took ${Date.now() - startTime}ms`, data);
	} else {
	logLSPMessage('send-response', message);
	}
	}

It seems to me that we should check for more than just message.error && message.error.data. If message.error.code and/or message.error.message is available then we should log those in the trace too.

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32602,
        "message": "Request client/registerCapability defines parameters by name but received parameters by position"
    }
}