feat(node-runtime-worker-thread): Advanced serialization for evaluation result COMPASS-4557 (#572)

* refactor(node-runtime-worker-thread): Simplify promisified type

* feat(node-runtime-worker-thread): Better serialization for errors thrown inside an rpc helper

* feat(node-runtime-worker-thread): Serialize errors thrown or returned by worker runtime

* refactor(node-runtime-worker-thread): Propagate serialization errors back to the client

* feat(node-runtime-worker-thread): Inspect complex, non-distinguishable shell result values before returning them

* fix(node-runtime-worker-thread): Use caller defined in beforeEach

* feat(node-runtime-worker-thread): Serialize shell-api return types for better output

* test(node-runtime-worker-thread): Add more tests for different types of return values

* fix(node-runtime-worker-thread): Fix bson missing from package-lock

* test(node-runtime-worker-thread): Fix anonymous fn inspection check for older node versions

* refactor(browser-runtime-core, node-runtime-worker-thread): Runtimes should return only serializable parts of shell result on evaluation

* refactor(node-runtime-worker-thread): Simplify {de}serialization implementation

* refactor(node-runtime-worker-thread): Move v8 {de}serialize to rpc helper as the sole user of those methods

* refactor(shell-api, browser-repl, cli-repl, node-runtime-worker-thread): Do not rely on non-enumerable, non-serializable properties in Cursor printable

* fix(shell-api): Fix failing tests

* refactor(node-runtime-worker-thread): Remove custom bson assertion

* fix(browser-repl): Fix more failing tests that depended on cursor printable

* refactor(node-runtime-worker-thread): Pick properties instead of omitting them

* refactor(node-runtime-worker-thread): Explicitly pass typed payloads in rpc helper internally

* chore(node-runtime-worker-thread): Remove confusing comments and replace them with (hopefully) less confusing ones

* refactor(node-runtime-worker-thread): Use enums instead of hardcoded strings

Author: gribnoysup

packages/browser-repl/src/components/types/cursor-iteration-result-output.spec.tsx

@@ -7,13 +7,18 @@ import { ObjectOutput } from './object-output';
 
 describe('CursorIterationResultOutput', () => {
   it('renders no ObjectOutput if value is empty', () => {
-    const wrapper = shallow(<CursorIterationResultOutput value={[]} />);
+    const printable = { documents: [], cursorHasMore: false };
+    const wrapper = shallow(<CursorIterationResultOutput value={printable} />);
 
     expect(wrapper.text()).to.contain('no cursor');
   });
 
   it('renders a ObjectOutput for each element in value', () => {
-    const wrapper = shallow(<CursorIterationResultOutput value={[{ doc: 1 }, { doc: 2 }]} />);
+    const printable = {
+      documents: [{ doc: 1 }, { doc: 2 }],
+      cursorHasMore: false
+    };
+    const wrapper = shallow(<CursorIterationResultOutput value={printable} />);
 
     expect(wrapper.find(ObjectOutput)).to.have.lengthOf(2);
   });

packages/browser-repl/src/components/types/cursor-iteration-result-output.tsx

@@ -8,7 +8,7 @@ export interface Document {
 }
 
 interface CursorIterationResultOutputProps {
-  value: Document[] & { cursorHasMore: boolean };
+  value: { documents: Document[]; cursorHasMore: boolean };
 }
 
 export class CursorIterationResultOutput extends Component<CursorIterationResultOutputProps> {
@@ -17,17 +17,22 @@ export class CursorIterationResultOutput extends Component<CursorIterationResult
   };
 
   render(): JSX.Element {
-    if (!this.props.value.length) {
-      return <div>{i18n.__('shell-api.classes.Cursor.iteration.no-cursor')}</div>;
+    if (!this.props.value.documents.length) {
+      return (
+        <div>{i18n.__('shell-api.classes.Cursor.iteration.no-cursor')}</div>
+      );
     }
 
     const more = this.props.value.cursorHasMore ?
       (<pre>{i18n.__('shell-api.classes.Cursor.iteration.type-it-for-more')}</pre>) :
       '';
-    return (<div>
-      {this.props.value.map(this.renderDocument)}
-      {more}
-    </div>);
+
+    return (
+      <div>
+        {this.props.value.documents.map(this.renderDocument)}
+        {more}
+      </div>
+    );
   }
 
   renderDocument = (document: Document, i: number): JSX.Element => {

packages/browser-repl/src/components/types/cursor-output.spec.tsx

@@ -7,22 +7,23 @@ import { CursorIterationResultOutput } from './cursor-iteration-result-output';
 
 describe('CursorOutput', () => {
   it('renders "no cursor" if value is empty', () => {
-    const wrapper = shallow(<CursorOutput value={[]} />);
+    const docs = { documents: [], cursorHasMore: false };
+    const wrapper = shallow(<CursorOutput value={docs} />);
 
     expect(wrapper.find(CursorIterationResultOutput)).to.have.lengthOf(0);
   });
 
 
   it('renders a CursorIterationResultOutput if value contains elements', () => {
-    const docs = [{ doc: 1 }, { doc: 2 }];
+    const docs = { documents: [{ doc: 1 }, { doc: 2 }], cursorHasMore: false };
     const wrapper = shallow(<CursorOutput value={docs} />);
 
     expect(wrapper.find(CursorIterationResultOutput).prop('value')).to.deep.equal(docs);
   });
 
   context('when value has more elements available', () => {
     it('prompts to type "it"', () => {
-      const docs = Object.assign([{}], { cursorHasMore: true });
+      const docs = { documents: [{}], cursorHasMore: true };
       const wrapper = mount(<CursorOutput value={docs} />);
 
       expect(wrapper.find(CursorIterationResultOutput).text()).to.contain('Type "it" for more');
@@ -31,7 +32,7 @@ describe('CursorOutput', () => {
 
   context('when value does not have more elements available', () => {
     it('does not prompt to type "it"', () => {
-      const docs = Object.assign([{}], { cursorHasMore: false });
+      const docs = { documents: [{}], cursorHasMore: false };
       const wrapper = mount(<CursorOutput value={docs} />);
 
       expect(wrapper.find(CursorIterationResultOutput).text()).not.to.contain('Type "it" for more');

packages/browser-repl/src/components/types/cursor-output.tsx

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import { CursorIterationResultOutput, Document } from './cursor-iteration-result-output';
 
 interface CursorOutputProps {
-  value: Document[] & { cursorHasMore: boolean };
+  value: { documents: Document[], cursorHasMore: boolean };
 }
 
 export class CursorOutput extends Component<CursorOutputProps> {
@@ -12,7 +12,7 @@ export class CursorOutput extends Component<CursorOutputProps> {
   };
 
   render(): JSX.Element {
-    if (!this.props.value.length) {
+    if (!this.props.value.documents.length) {
       return <pre/>;
     }
 

packages/browser-repl/src/iframe-runtime/iframe-runtime.ts

@@ -4,26 +4,27 @@ import {
 
 import {
   Runtime,
+  RuntimeEvaluationListener,
+  RuntimeEvaluationResult,
   Completion,
   OpenContextRuntime
 } from '@mongosh/browser-runtime-core';
 
 import { ServiceProvider } from '@mongosh/service-provider-core';
-import { ShellResult, EvaluationListener } from '@mongosh/shell-evaluator';
 
 export class IframeRuntime implements Runtime {
   private openContextRuntime: OpenContextRuntime | null = null;
   private readyPromise: Promise<void> | null = null;
   private iframe: HTMLIFrameElement | null = null;
   private container: HTMLDivElement | null = null;
   private serviceProvider: ServiceProvider;
-  private evaluationListener: EvaluationListener | null = null;
+  private evaluationListener: RuntimeEvaluationListener | null = null;
 
   constructor(serviceProvider: ServiceProvider) {
     this.serviceProvider = serviceProvider;
   }
 
-  setEvaluationListener(listener: EvaluationListener): EvaluationListener | null {
+  setEvaluationListener(listener: RuntimeEvaluationListener): RuntimeEvaluationListener | null {
     const prev = this.evaluationListener;
     this.evaluationListener = listener;
     if (this.openContextRuntime) {
@@ -32,7 +33,7 @@ export class IframeRuntime implements Runtime {
     return prev;
   }
 
-  async evaluate(code: string): Promise<ShellResult> {
+  async evaluate(code: string): Promise<RuntimeEvaluationResult> {
     const runtime = await this.initialize();
     return await runtime.evaluate(code);
   }

packages/browser-runtime-core/src/index.ts

@@ -1,4 +1,4 @@
-export { Runtime } from './runtime';
+export { Runtime, RuntimeEvaluationListener, RuntimeEvaluationResult } from './runtime';
 export { ContextValue, InterpreterEnvironment } from './interpreter';
 export { OpenContextRuntime } from './open-context-runtime';
 export { Autocompleter, Completion } from './autocompleter/autocompleter';

packages/browser-runtime-core/src/open-context-runtime.ts

@@ -2,11 +2,15 @@ import { Completion } from './autocompleter/autocompleter';
 import { ServiceProvider } from '@mongosh/service-provider-core';
 import { ShellApiAutocompleter } from './autocompleter/shell-api-autocompleter';
 import { Interpreter, InterpreterEnvironment } from './interpreter';
-import { Runtime } from './runtime';
+import {
+  Runtime,
+  RuntimeEvaluationResult,
+  RuntimeEvaluationListener
+} from './runtime';
 import { EventEmitter } from 'events';
-import { ShellInternalState, ShellResult } from '@mongosh/shell-api';
+import { ShellInternalState } from '@mongosh/shell-api';
 
-import { ShellEvaluator, EvaluationListener } from '@mongosh/shell-evaluator';
+import { ShellEvaluator } from '@mongosh/shell-evaluator';
 import type { MongoshBus } from '@mongosh/types';
 
 /**
@@ -23,7 +27,7 @@ export class OpenContextRuntime implements Runtime {
   private autocompleter: ShellApiAutocompleter | null = null;
   private shellEvaluator: ShellEvaluator;
   private internalState: ShellInternalState;
-  private evaluationListener: EvaluationListener | null = null;
+  private evaluationListener: RuntimeEvaluationListener | null = null;
   private updatedConnectionInfo = false;
 
   constructor(
@@ -48,17 +52,18 @@ export class OpenContextRuntime implements Runtime {
     return this.autocompleter.getCompletions(code);
   }
 
-  async evaluate(code: string): Promise<ShellResult> {
+  async evaluate(code: string): Promise<RuntimeEvaluationResult> {
     const evalFn = this.interpreter.evaluate.bind(this.interpreter);
-    return await this.shellEvaluator.customEval(
+    const { type, printable, source } = await this.shellEvaluator.customEval(
       evalFn,
       code,
       this.interpreterEnvironment.getContextObject(),
       ''
     );
+    return { type, printable, source };
   }
 
-  setEvaluationListener(listener: EvaluationListener): EvaluationListener | null {
+  setEvaluationListener(listener: RuntimeEvaluationListener): RuntimeEvaluationListener | null {
     const prev = this.evaluationListener;
     this.evaluationListener = listener;
     this.internalState.setEvaluationListener(listener);

packages/browser-runtime-core/src/runtime.ts

@@ -3,23 +3,34 @@ import { ShellResult, EvaluationListener } from '@mongosh/shell-evaluator';
 
 export type ContextValue = any;
 
+export type RuntimeEvaluationResult = Pick<
+  ShellResult,
+  'type' | 'printable' | 'source'
+>;
+
+export interface RuntimeEvaluationListener extends EvaluationListener {
+  onPrint?: (value: RuntimeEvaluationResult[]) => Promise<void> | void;
+}
+
 export interface Runtime {
   /**
    * Sets a listener for certain events, e.g. onPrint() when print() is called
    * in the shell.
    *
-   * @param {EvaluationListener} listener - The new listener.
-   * @return {EvaluationListener | null} The previous listener, if any.
+   * @param {RuntimeEvaluationListener} listener - The new listener.
+   * @return {RuntimeEvaluationListener | null} The previous listener, if any.
    */
-  setEvaluationListener(listener: EvaluationListener): EvaluationListener | null;
+  setEvaluationListener(
+    listener: RuntimeEvaluationListener
+  ): RuntimeEvaluationListener | null;
 
   /**
    * Evaluates code
    *
    * @param {string} code - A string of code
-   * @return {Promise<ShellResult>} the result of the evaluation
+   * @return {Promise<RuntimeEvaluationResult>} the result of the evaluation
    */
-  evaluate(code: string): Promise<ShellResult>;
+  evaluate(code: string): Promise<RuntimeEvaluationResult>;
 
   /**
    * Get shell api completions give a code prefix

packages/browser-runtime-electron/src/electron-runtime.spec.ts

@@ -8,12 +8,12 @@ import { CliServiceProvider } from '@mongosh/service-provider-server';
 import { bson } from '@mongosh/service-provider-core';
 import { ElectronRuntime } from './electron-runtime';
 import { EventEmitter } from 'events';
-import { EvaluationListener } from '@mongosh/shell-evaluator';
+import { RuntimeEvaluationListener } from '@mongosh/browser-runtime-core';
 
 describe('Electron runtime', function() {
   let serviceProvider: SinonStubbedInstance<CliServiceProvider>;
   let messageBus: SinonStubbedInstance<EventEmitter>;
-  let evaluationListener: SinonStubbedInstance<EvaluationListener>;
+  let evaluationListener: SinonStubbedInstance<RuntimeEvaluationListener>;
   let electronRuntime: ElectronRuntime;
 
   beforeEach(async() => {

packages/browser-runtime-electron/src/electron-runtime.ts

@@ -6,11 +6,12 @@ import {
 import {
   Runtime,
   OpenContextRuntime,
-  Completion
+  Completion,
+  RuntimeEvaluationListener,
+  RuntimeEvaluationResult
 } from '@mongosh/browser-runtime-core';
 
 import { ServiceProvider } from '@mongosh/service-provider-core';
-import { ShellResult, EvaluationListener } from '@mongosh/shell-evaluator';
 import type { MongoshBus } from '@mongosh/types';
 
 declare const __webpack_require__: any;
@@ -42,11 +43,11 @@ export class ElectronRuntime implements Runtime {
     );
   }
 
-  setEvaluationListener(listener: EvaluationListener): EvaluationListener | null {
+  setEvaluationListener(listener: RuntimeEvaluationListener): RuntimeEvaluationListener | null {
     return this.openContextRuntime.setEvaluationListener(listener);
   }
 
-  async evaluate(code: string): Promise<ShellResult> {
+  async evaluate(code: string): Promise<RuntimeEvaluationResult> {
     return await this.openContextRuntime.evaluate(code);
   }