feat(transport): show contents of invalid RPC message

Problem:
Exception does not give any hint about the contents of an invalid RPC
message:

    "TypeError: expected msgpack result to be array"

Solution:
Show the payload contents in the error message. Example:

    "TypeError: expected msgpack array, got: …"

Author: justinmk

CHANGELOG.md

@@ -8,6 +8,7 @@
   makes it more useful for GUIs where `$PATH` is often different than the user's
   shell. #267
 - `findNvim` supports optional `paths` and `dirs` parameters.
+- Invalid RPC error now shows the contents of the invalid RPC message. #404
 
 ## [5.2.1](https://github.com/neovim/node-client/compare/v5.2.0...v5.2.1)
 

packages/neovim/src/utils/transport.test.ts

@@ -0,0 +1,30 @@
+/* eslint-env jest */
+import { EventEmitter } from 'node:events';
+import { Readable, Writable } from 'node:stream';
+import * as msgpack from '@msgpack/msgpack';
+import { attach } from '../attach/attach';
+import { exportsForTesting } from './transport';
+
+describe('transport', () => {
+  it('throws on invalid RPC message', done => {
+    const invalidPayload = { bogus: 'nonsense' };
+    const onTransportFail: EventEmitter = exportsForTesting.onTransportFail;
+    onTransportFail.on('fail', (errMsg: string) => {
+      expect(errMsg).toEqual(
+        "invalid msgpack-RPC message: expected array, got: { bogus: 'nonsense' }"
+      );
+      done();
+    });
+
+    // Create fake reader/writer and send a (broken) message.
+    const fakeReader = new Readable({ read() {} });
+    const fakeWriter = new Writable({ write() {} });
+
+    const nvim = attach({ reader: fakeReader, writer: fakeWriter });
+    void nvim; // eslint-disable-line no-void
+
+    // Simulate an invalid message on the channel.
+    const msg = msgpack.encode(invalidPayload);
+    fakeReader.push(Buffer.from(msg.buffer, msg.byteOffset, msg.byteLength));
+  });
+});

packages/neovim/src/utils/transport.ts

@@ -13,6 +13,14 @@ import {
 } from '@msgpack/msgpack';
 import { Metadata } from '../api/types';
 
+export let exportsForTesting: any; // eslint-disable-line import/no-mutable-exports
+// jest sets NODE_ENV=test.
+if (process.env.NODE_ENV === 'test') {
+  exportsForTesting = {
+    onTransportFail: new EventEmitter(),
+  };
+}
+
 class Response {
   private requestId: number;
 
@@ -122,16 +130,25 @@ class Transport extends EventEmitter {
                 breakLength: 500,
               });
             } catch (error) {
-              console.error('Failed to inspect value: ', error);
+              // Do nothing.
             }
-            throw new TypeError(
-              `Expected msgpack result to be an array, but got ${valstr}`
-            );
+
+            const errMsg = `invalid msgpack-RPC message: expected array, got: ${valstr}`;
+            const onFail: EventEmitter = exportsForTesting?.onTransportFail;
+            if (onFail) {
+              // HACK: for testing only.
+              // TODO(justinmk): let the tests explicitly drive the messages.
+              onFail.emit('fail', errMsg);
+              return;
+            }
+            throw new TypeError(errMsg);
           }
+
           this.parseMessage(resolved.value);
-          return resolveGeneratorRecursively(iter);
+          resolveGeneratorRecursively(iter);
+          return;
         }
-        return Promise.resolve();
+        Promise.resolve();
       });
     };