From 83531876d9c1e058e410a21a1bd109513eaebbbd Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 2 Sep 2020 19:52:09 +0800 Subject: [PATCH 1/2] fix: show the detail error messages from worker --- .../client/src/recoilModel/dispatchers/lg.ts | 174 +++++++++++------- .../client/src/recoilModel/dispatchers/lu.ts | 93 ++++++---- .../src/recoilModel/parsers/baseWorker.ts | 2 +- .../parsers/workers/lgParser.worker.ts | 2 +- .../parsers/workers/luParser.worker.ts | 2 +- .../parsers/workers/qnaParser.worker.ts | 2 +- 6 files changed, 167 insertions(+), 108 deletions(-) diff --git a/Composer/packages/client/src/recoilModel/dispatchers/lg.ts b/Composer/packages/client/src/recoilModel/dispatchers/lg.ts index a294e9d86a..93afaaed1c 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/lg.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/lg.ts @@ -8,6 +8,7 @@ import formatMessage from 'format-message'; import { getBaseName, getExtension } from '../../utils/fileUtil'; +import { setError } from './shared'; import LgWorker from './../parsers/lgWorker'; import { lgFilesState, localeState, settingsState, projectIdState } from './../atoms/botState'; @@ -59,7 +60,6 @@ export const updateLgFileState = async (projectId: string, lgFiles: LgFile[], up deletedTemplates.map(({ name }) => name), lgFiles )) as LgFile; - changes.push(newLgFile); } } @@ -115,26 +115,39 @@ export const removeLgFileState = async (callbackHelpers: CallbackInterface, { id export const lgDispatcher = () => { const createLgFile = useRecoilCallback( (callbackHelpers: CallbackInterface) => async ({ id, content }: { id: string; content: string }) => { - await createLgFileState(callbackHelpers, { id, content }); + try { + await createLgFileState(callbackHelpers, { id, content }); + } catch (error) { + setError(callbackHelpers, error); + } } ); const removeLgFile = useRecoilCallback((callbackHelpers: CallbackInterface) => async ({ id }: { id: string }) => { - await removeLgFileState(callbackHelpers, { id }); + try { + await removeLgFileState(callbackHelpers, { id }); + } catch (error) { + setError(callbackHelpers, error); + } }); const updateLgFile = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ id, content }: { id: string; content: string }) => { + (callbackHelpers: CallbackInterface) => async ({ id, content }: { id: string; content: string }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); - const updatedFile = (await LgWorker.parse(projectId, id, content, lgFiles)) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + try { + const updatedFile = (await LgWorker.parse(projectId, id, content, lgFiles)) as LgFile; + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); const updateLgTemplate = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, templateName, template, @@ -143,109 +156,129 @@ export const lgDispatcher = () => { templateName: string; template: LgTemplate; }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; const sameIdOtherLocaleFiles = lgFiles.filter((file) => getBaseName(file.id) === getBaseName(id)); - if (template.name !== templateName) { - // name change, need update cross multi locale file. - const changes: LgFile[] = []; - for (const item of sameIdOtherLocaleFiles) { + try { + if (template.name !== templateName) { + // name change, need update cross multi locale file. + const changes: LgFile[] = []; + + for (const item of sameIdOtherLocaleFiles) { + const updatedFile = (await LgWorker.updateTemplate( + projectId, + item, + templateName, + { name: template.name }, + lgFiles + )) as LgFile; + changes.push(updatedFile); + } + + set(lgFilesState, (lgFiles) => { + return lgFiles.map((file) => { + const changedFile = changes.find(({ id }) => id === file.id); + return changedFile ? changedFile : file; + }); + }); + } else { + // body change, only update current locale file const updatedFile = (await LgWorker.updateTemplate( projectId, - item, + lgFile, templateName, - { name: template.name }, + { body: template.body }, lgFiles )) as LgFile; - changes.push(updatedFile); - } - - set(lgFilesState, (lgFiles) => { - return lgFiles.map((file) => { - const changedFile = changes.find(({ id }) => id === file.id); - return changedFile ? changedFile : file; - }); - }); - } else { - // body change, only update current locale file - const updatedFile = (await LgWorker.updateTemplate( - projectId, - lgFile, - templateName, - { body: template.body }, - lgFiles - )) as LgFile; - set(lgFilesState, (lgFiles) => { - return lgFiles.map((file) => { - return file.id === id ? updatedFile : file; + set(lgFilesState, (lgFiles) => { + return lgFiles.map((file) => { + return file.id === id ? updatedFile : file; + }); }); - }); + } + } catch (error) { + setError(callbackHelpers, error); } } ); const createLgTemplate = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ id, template }: { id: string; template: LgTemplate }) => { + (callbackHelpers: CallbackInterface) => async ({ id, template }: { id: string; template: LgTemplate }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; - const updatedFile = (await LgWorker.addTemplate(projectId, lgFile, template, lgFiles)) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + try { + const updatedFile = (await LgWorker.addTemplate(projectId, lgFile, template, lgFiles)) as LgFile; + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); const createLgTemplates = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ id, templates }: { id: string; templates: LgTemplate[] }) => { + (callbackHelpers: CallbackInterface) => async ({ id, templates }: { id: string; templates: LgTemplate[] }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; - const updatedFile = (await LgWorker.addTemplates(projectId, lgFile, templates, lgFiles)) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + try { + const updatedFile = (await LgWorker.addTemplates(projectId, lgFile, templates, lgFiles)) as LgFile; + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); const removeLgTemplate = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ id, templateName }: { id: string; templateName: string }) => { + (callbackHelpers: CallbackInterface) => async ({ id, templateName }: { id: string; templateName: string }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; - const updatedFile = (await LgWorker.removeTemplate(projectId, lgFile, templateName, lgFiles)) as LgFile; + try { + const updatedFile = (await LgWorker.removeTemplate(projectId, lgFile, templateName, lgFiles)) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); const removeLgTemplates = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ - id, - templateNames, - }: { - id: string; - templateNames: string[]; - }) => { + (callbackHelpers: CallbackInterface) => async ({ id, templateNames }: { id: string; templateNames: string[] }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; - const updatedFile = (await LgWorker.removeTemplates(projectId, lgFile, templateNames, lgFiles)) as LgFile; + try { + const updatedFile = (await LgWorker.removeTemplates(projectId, lgFile, templateNames, lgFiles)) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); const copyLgTemplate = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, fromTemplateName, toTemplateName, @@ -254,19 +287,24 @@ export const lgDispatcher = () => { fromTemplateName: string; toTemplateName: string; }) => { + const { set, snapshot } = callbackHelpers; const lgFiles = await snapshot.getPromise(lgFilesState); const projectId = await snapshot.getPromise(projectIdState); const lgFile = lgFiles.find((file) => file.id === id); if (!lgFile) return lgFiles; - const updatedFile = (await LgWorker.copyTemplate( - projectId, - lgFile, - fromTemplateName, - toTemplateName, - lgFiles - )) as LgFile; - const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); - set(lgFilesState, updatedFiles); + try { + const updatedFile = (await LgWorker.copyTemplate( + projectId, + lgFile, + fromTemplateName, + toTemplateName, + lgFiles + )) as LgFile; + const updatedFiles = await updateLgFileState(projectId, lgFiles, updatedFile); + set(lgFilesState, updatedFiles); + } catch (error) { + setError(callbackHelpers, error); + } } ); diff --git a/Composer/packages/client/src/recoilModel/dispatchers/lu.ts b/Composer/packages/client/src/recoilModel/dispatchers/lu.ts index b74739658c..f85715e53f 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/lu.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/lu.ts @@ -11,6 +11,8 @@ import { getBaseName, getExtension } from '../../utils/fileUtil'; import luFileStatusStorage from '../../utils/luFileStatusStorage'; import { luFilesState, projectIdState, localeState, settingsState } from '../atoms/botState'; +import { setError } from './shared'; + const intentIsNotEmpty = ({ Name, Body }) => { return !!Name && !!Body; }; @@ -113,7 +115,7 @@ export const removeLuFileState = async (callbackHelpers: CallbackInterface, { id export const luDispatcher = () => { const updateLuFile = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, content, projectId, @@ -122,15 +124,20 @@ export const luDispatcher = () => { content: string; projectId: string; }) => { + const { set, snapshot } = callbackHelpers; const luFiles = await snapshot.getPromise(luFilesState); - const updatedFile = (await luWorker.parse(id, content)) as LuFile; - const result = await updateLuFileState(luFiles, updatedFile, projectId); - set(luFilesState, result); + try { + const updatedFile = (await luWorker.parse(id, content)) as LuFile; + const result = await updateLuFileState(luFiles, updatedFile, projectId); + set(luFilesState, result); + } catch (error) { + setError(callbackHelpers, error); + } } ); const updateLuIntent = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, intentName, intent, @@ -139,40 +146,44 @@ export const luDispatcher = () => { intentName: string; intent: LuIntentSection; }) => { + const { set, snapshot } = callbackHelpers; const luFiles = await snapshot.getPromise(luFilesState); const luFile = luFiles.find((temp) => temp.id === id); if (!luFile) return luFiles; - - const sameIdOtherLocaleFiles = luFiles.filter((file) => getBaseName(file.id) === getBaseName(id)); - - // name change, need update cross multi locale file. - if (intent.Name !== intentName) { - const changes: LuFile[] = []; - for (const item of sameIdOtherLocaleFiles) { - const updatedFile = (await luWorker.updateIntent(item, intentName, { Name: intent.Name })) as LuFile; - changes.push(updatedFile); - } - - set(luFilesState, (luFiles) => { - return luFiles.map((file) => { - const changedFile = changes.find(({ id }) => id === file.id); - return changedFile ? changedFile : file; + try { + const sameIdOtherLocaleFiles = luFiles.filter((file) => getBaseName(file.id) === getBaseName(id)); + + // name change, need update cross multi locale file. + if (intent.Name !== intentName) { + const changes: LuFile[] = []; + for (const item of sameIdOtherLocaleFiles) { + const updatedFile = (await luWorker.updateIntent(item, intentName, { Name: intent.Name })) as LuFile; + changes.push(updatedFile); + } + + set(luFilesState, (luFiles) => { + return luFiles.map((file) => { + const changedFile = changes.find(({ id }) => id === file.id); + return changedFile ? changedFile : file; + }); }); - }); - // body change, only update current locale file - } else { - const updatedFile = (await luWorker.updateIntent(luFile, intentName, { Body: intent.Body })) as LuFile; - set(luFilesState, (luFiles) => { - return luFiles.map((file) => { - return file.id === id ? updatedFile : file; + // body change, only update current locale file + } else { + const updatedFile = (await luWorker.updateIntent(luFile, intentName, { Body: intent.Body })) as LuFile; + set(luFilesState, (luFiles) => { + return luFiles.map((file) => { + return file.id === id ? updatedFile : file; + }); }); - }); + } + } catch (error) { + setError(callbackHelpers, error); } } ); const createLuIntent = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, intent, projectId, @@ -181,17 +192,22 @@ export const luDispatcher = () => { intent: LuIntentSection; projectId: string; }) => { + const { set, snapshot } = callbackHelpers; const luFiles = await snapshot.getPromise(luFilesState); const file = luFiles.find((temp) => temp.id === id); if (!file) return luFiles; - const updatedFile = (await luWorker.addIntent(file, intent)) as LuFile; - const result = await updateLuFileState(luFiles, updatedFile, projectId); - set(luFilesState, result); + try { + const updatedFile = (await luWorker.addIntent(file, intent)) as LuFile; + const result = await updateLuFileState(luFiles, updatedFile, projectId); + set(luFilesState, result); + } catch (error) { + setError(callbackHelpers, error); + } } ); const removeLuIntent = useRecoilCallback( - ({ set, snapshot }: CallbackInterface) => async ({ + (callbackHelpers: CallbackInterface) => async ({ id, intentName, projectId, @@ -200,12 +216,17 @@ export const luDispatcher = () => { intentName: string; projectId: string; }) => { + const { set, snapshot } = callbackHelpers; const luFiles = await snapshot.getPromise(luFilesState); const file = luFiles.find((temp) => temp.id === id); if (!file) return luFiles; - const updatedFile = (await luWorker.removeIntent(file, intentName)) as LuFile; - const result = await updateLuFileState(luFiles, updatedFile, projectId); - set(luFilesState, result); + try { + const updatedFile = (await luWorker.removeIntent(file, intentName)) as LuFile; + const result = await updateLuFileState(luFiles, updatedFile, projectId); + set(luFilesState, result); + } catch (error) { + setError(callbackHelpers, error); + } } ); diff --git a/Composer/packages/client/src/recoilModel/parsers/baseWorker.ts b/Composer/packages/client/src/recoilModel/parsers/baseWorker.ts index 9163ee9a61..85079ffc97 100644 --- a/Composer/packages/client/src/recoilModel/parsers/baseWorker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/baseWorker.ts @@ -38,7 +38,7 @@ export class BaseWorker { const { id, error, payload } = msg.data; if (error) { const reject = this.rejects[id]; - reject(error); + reject({ message: error }); } else { const resolve = this.resolves[id]; if (resolve) resolve(payload); diff --git a/Composer/packages/client/src/recoilModel/parsers/workers/lgParser.worker.ts b/Composer/packages/client/src/recoilModel/parsers/workers/lgParser.worker.ts index 34abb529c0..169e3949e2 100644 --- a/Composer/packages/client/src/recoilModel/parsers/workers/lgParser.worker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/workers/lgParser.worker.ts @@ -236,6 +236,6 @@ ctx.onmessage = function (event) { ctx.postMessage({ id: msg.id, payload }); } catch (error) { - ctx.postMessage({ id: msg.id, error }); + ctx.postMessage({ id: msg.id, error: error.message }); } }; diff --git a/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts b/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts index e4e0556cc0..ed4a8b8437 100644 --- a/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts @@ -101,6 +101,6 @@ ctx.onmessage = function (msg) { ctx.postMessage({ id, payload }); } catch (error) { - ctx.postMessage({ id, error }); + ctx.postMessage({ id, error: error.message }); } }; diff --git a/Composer/packages/client/src/recoilModel/parsers/workers/qnaParser.worker.ts b/Composer/packages/client/src/recoilModel/parsers/workers/qnaParser.worker.ts index 1d265a3638..7dc3481f4a 100644 --- a/Composer/packages/client/src/recoilModel/parsers/workers/qnaParser.worker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/workers/qnaParser.worker.ts @@ -35,6 +35,6 @@ ctx.onmessage = function (msg) { } ctx.postMessage({ id: msgId, payload: result }); } catch (error) { - ctx.postMessage({ id: msgId, error }); + ctx.postMessage({ id: msgId, error: error.message }); } }; From d9aa9a96625a8c6041d02e2032c8c9537bc5f3d7 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 2 Sep 2020 20:49:00 +0800 Subject: [PATCH 2/2] fix test --- .../client/src/recoilModel/parsers/__test__/baseWorker.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/recoilModel/parsers/__test__/baseWorker.test.ts b/Composer/packages/client/src/recoilModel/parsers/__test__/baseWorker.test.ts index d90fb4d9fc..3370133c5a 100644 --- a/Composer/packages/client/src/recoilModel/parsers/__test__/baseWorker.test.ts +++ b/Composer/packages/client/src/recoilModel/parsers/__test__/baseWorker.test.ts @@ -34,7 +34,7 @@ describe('test base worker', () => { try { await testWorker.sendMsg('error', { test: '1' }); } catch (error) { - expect(error).toBe('error'); + expect(error.message).toBe('error'); } });