Merge #404 show contents of invalid RPC message

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/attach.ts

@@ -1,4 +1,4 @@
-import { attach } from '../attach';
+import { attach } from '../attach/attach';
 import { loadPlugin, LoadPluginOptions } from './factory';
 import { NvimPlugin } from './NvimPlugin';
 

packages/neovim/src/host/index.ts

@@ -1,4 +1,4 @@
-export { attach } from './attach';
+export { attach } from './attach/attach';
 export { Neovim, NeovimClient, Buffer, Tabpage, Window } from './api/index';
 export { Plugin, Function, Autocmd, Command } from './plugin';
 export { NvimPlugin } from './host/NvimPlugin';

packages/neovim/src/index.ts

@@ -4,7 +4,7 @@ import * as jest from '@jest/globals';
 import * as fs from 'node:fs';
 import * as path from 'node:path';
 import { NeovimClient } from './api/client';
-import { attach } from './attach';
+import { attach } from './attach/attach';
 import { findNvim } from './utils/findNvim';
 import { getLogger } from './utils/logger';
 

packages/neovim/src/testUtil.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.test.ts

@@ -3,6 +3,7 @@
  */
 
 import { EventEmitter } from 'node:events';
+import { inspect } from 'node:util';
 
 import {
   encode,
@@ -12,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;
 
@@ -111,12 +120,35 @@ class Transport extends EventEmitter {
       iter.next().then(resolved => {
         if (!resolved.done) {
           if (!Array.isArray(resolved.value)) {
-            throw new TypeError('expected msgpack result to be array');
+            let valstr = '?';
+            try {
+              valstr = inspect(resolved.value, {
+                sorted: true,
+                maxArrayLength: 10,
+                maxStringLength: 500,
+                compact: true,
+                breakLength: 500,
+              });
+            } catch (error) {
+              // Do nothing.
+            }
+
+            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();
       });
     };