From 76602ba7cdbbbe29b28e5664288c1aae0ebcd679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Thu, 5 Sep 2019 13:57:58 +0200 Subject: [PATCH 1/2] fix(rendering): uncaught exception --- src/components/JsonSchemaViewer.tsx | 20 ++++++++++---------- src/workers/messages.ts | 23 +++++++++++++++++++++-- src/workers/schema.ts | 27 ++++++++++++++++++--------- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/components/JsonSchemaViewer.tsx b/src/components/JsonSchemaViewer.tsx index 571000a4..e20451a9 100644 --- a/src/components/JsonSchemaViewer.tsx +++ b/src/components/JsonSchemaViewer.tsx @@ -1,7 +1,7 @@ import { TreeStore } from '@stoplight/tree-list'; import { Intent, Spinner } from '@stoplight/ui-kit'; import cn from 'classnames'; -import { runInAction } from 'mobx'; +import { action, runInAction } from 'mobx'; import * as React from 'react'; import SchemaWorker, { WebWorker } from 'web-worker:../workers/schema.ts'; @@ -10,7 +10,7 @@ import { GoToRefHandler, RowRenderer, SchemaTreeListNode } from '../types'; import { isCombiner } from '../utils/isCombiner'; import { isSchemaViewerEmpty } from '../utils/isSchemaViewerEmpty'; import { renderSchema } from '../utils/renderSchema'; -import { ComputeSchemaMessage, RenderedSchemaMessage } from '../workers/messages'; +import { ComputeSchemaMessageData, isRenderedSchemaMessage } from '../workers/messages'; import { SchemaTree } from './SchemaTree'; export type FallbackComponent = React.ComponentType<{ error: Error | null }>; @@ -68,17 +68,17 @@ export class JsonSchemaViewerComponent extends React.PureComponent { - if (!message.data || !('instanceId' in message.data) || !('nodes' in message.data)) return; - const data = message.data as RenderedSchemaMessage; + protected handleWorkerMessage = action<(message: MessageEvent) => void>(message => { + if (!isRenderedSchemaMessage(message)) return; + const { data } = message; if (data.instanceId === this.instanceId) { - runInAction(() => { - this.setState({ computing: false }); + this.setState({ computing: false }); + if (data.error === null) { this.treeStore.nodes = data.nodes; - }); + } } - }; + }); protected prerenderSchema() { const schema = this.schema; @@ -136,7 +136,7 @@ export class JsonSchemaViewerComponent extends React.PureComponent extends Omit { + data: T; +} + +export type ComputeSchemaMessageData = { instanceId: string; schema: JSONSchema4; mergeAllOf: boolean; }; -export type RenderedSchemaMessage = { +export type RenderedSchemaMessageData = RenderedSchemaSuccessMessageData | RenderedSchemaErrorMessageData; + +export type RenderedSchemaSuccessMessageData = { instanceId: string; + error: null; nodes: SchemaTreeListNode[]; }; + +export type RenderedSchemaErrorMessageData = { + instanceId: string; + error: string; + nodes: null; +}; + +export const isRenderedSchemaMessage = (message: MessageEvent): message is WorkerMessageEvent => message.data && 'instanceId' in message.data && 'nodes' in message.data; + +export const isComputeSchemaMessage = (message: MessageEvent): message is WorkerMessageEvent => message.data && 'instanceId' in message.data && 'schema' in message.data; + + diff --git a/src/workers/schema.ts b/src/workers/schema.ts index d942438a..ecc1750b 100644 --- a/src/workers/schema.ts +++ b/src/workers/schema.ts @@ -1,17 +1,26 @@ import { mergeAllOf } from '../utils/mergeAllOf'; import { renderSchema } from '../utils/renderSchema'; -import { ComputeSchemaMessage } from './messages'; +import { isComputeSchemaMessage, RenderedSchemaErrorMessageData, RenderedSchemaSuccessMessageData } from './messages'; declare const self: DedicatedWorkerGlobalScope; self.addEventListener('message', e => { - const msg = e.data; - if (typeof msg !== 'object' || msg === null || !('schema' in msg) || !('instanceId' in msg)) return; + if (!isComputeSchemaMessage(e)) return; + const { + data: { instanceId, schema, mergeAllOf: shouldMergeAllOf }, + } = e; - const schema = (msg as ComputeSchemaMessage).schema; - - self.postMessage({ - instanceId: (msg as ComputeSchemaMessage).instanceId, - nodes: Array.from(renderSchema((msg as ComputeSchemaMessage).mergeAllOf ? mergeAllOf(schema) : schema)), - }); + try { + self.postMessage({ + instanceId, + error: null, + nodes: Array.from(renderSchema(shouldMergeAllOf ? mergeAllOf(schema) : schema)), + } as RenderedSchemaSuccessMessageData); + } catch (ex) { + self.postMessage({ + instanceId, + error: ex.message, + nodes: null, + } as RenderedSchemaErrorMessageData); + } }); From fc56225a5d9befb17f03e781516aa6264f4740c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Thu, 5 Sep 2019 21:18:09 +0200 Subject: [PATCH 2/2] feat: expose onError --- src/components/JsonSchemaViewer.tsx | 16 ++++++-- .../__tests__/JsonSchemaViewer.spec.tsx | 40 +++++++++++++++---- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/components/JsonSchemaViewer.tsx b/src/components/JsonSchemaViewer.tsx index e20451a9..a0a74f16 100644 --- a/src/components/JsonSchemaViewer.tsx +++ b/src/components/JsonSchemaViewer.tsx @@ -32,7 +32,11 @@ export interface IJsonSchemaViewer { rowRenderer?: RowRenderer; } -export class JsonSchemaViewerComponent extends React.PureComponent { +export interface IJsonSchemaViewerComponent extends Omit { + onError(err: Error): void; +} + +export class JsonSchemaViewerComponent extends React.PureComponent { protected treeStore: TreeStore; protected instanceId: string; protected schemaWorker?: WebWorker; @@ -41,7 +45,7 @@ export class JsonSchemaViewerComponent extends React.PureComponent { + this.setState({ error }); + }; + public render() { const { FallbackComponent: Fallback = JsonSchemaFallbackComponent, ...props } = this.props; if (this.state.error) { return ; } - return ; + return ; } } diff --git a/src/components/__tests__/JsonSchemaViewer.spec.tsx b/src/components/__tests__/JsonSchemaViewer.spec.tsx index edf3fc44..0418dd3f 100644 --- a/src/components/__tests__/JsonSchemaViewer.spec.tsx +++ b/src/components/__tests__/JsonSchemaViewer.spec.tsx @@ -47,19 +47,21 @@ describe('JSON Schema Viewer component', () => { test('should render empty message if schema is empty', () => { (isSchemaViewerEmpty as jest.Mock).mockReturnValue(true); - const wrapper = shallow(); + const wrapper = shallow(); expect(isSchemaViewerEmpty).toHaveBeenCalledWith({}); expect(wrapper.find(SchemaTree)).not.toExist(); }); test('should render SchemaView if schema is provided', () => { - const wrapper = shallow(); + const wrapper = shallow(); expect(isSchemaViewerEmpty).toHaveBeenCalledWith(schema); expect(wrapper.find(SchemaTree)).toExist(); }); test('should not perform full processing in a worker if provided schema has fewer nodes than maxRows', () => { - const wrapper = shallow(); + const wrapper = shallow( + , + ); expect(SchemaWorker.prototype.postMessage).not.toHaveBeenCalled(); expect(wrapper.instance()).toHaveProperty('treeStore.nodes.length', 4); }); @@ -79,7 +81,7 @@ describe('JSON Schema Viewer component', () => { ], }; - shallow(); + shallow(); expect(SchemaWorker.prototype.postMessage).toHaveBeenCalledWith({ instanceId: expect.any(String), @@ -103,13 +105,15 @@ describe('JSON Schema Viewer component', () => { ], }; - shallow(); + shallow(); expect(SchemaWorker.prototype.postMessage).not.toHaveBeenCalledWith(); }); test('should pre-render maxRows nodes and perform full processing in a worker if provided schema has more nodes than maxRows', () => { - const wrapper = shallow(); + const wrapper = shallow( + , + ); expect(SchemaWorker.prototype.postMessage).toHaveBeenCalledWith({ instanceId: expect.any(String), mergeAllOf: true, @@ -152,6 +156,7 @@ describe('JSON Schema Viewer component', () => { SchemaWorker.prototype.addEventListener.mock.calls[0][1]({ data: { instanceId: SchemaWorker.prototype.postMessage.mock.calls[0][0].instanceId, + error: null, nodes, }, }); @@ -161,14 +166,33 @@ describe('JSON Schema Viewer component', () => { test('should render all nodes on main thread when worker cannot be spawned regardless of maxRows or schema', () => { SchemaWorker.prototype.isShim = true; - const wrapper = shallow(); + const wrapper = shallow( + , + ); expect(SchemaWorker.prototype.postMessage).not.toHaveBeenCalled(); expect(wrapper.instance()).toHaveProperty('treeStore.nodes.length', 4); }); + test('should handle exceptions that may occur during full rendering', () => { + const onError = jest.fn(); + shallow(); + + SchemaWorker.prototype.addEventListener.mock.calls[0][1]({ + data: { + instanceId: SchemaWorker.prototype.postMessage.mock.calls[0][0].instanceId, + error: 'error occurred', + nodes: null, + }, + }); + + expect(onError).toHaveBeenCalledWith(new Error('error occurred')); + }); + test('should not apply result of full processing in a worker if instance ids do not match', () => { - const wrapper = shallow(); + const wrapper = shallow( + , + ); expect(SchemaWorker.prototype.postMessage).toHaveBeenCalledWith({ instanceId: expect.any(String), mergeAllOf: true,