From 806eba6b0c1229dad81252b621211398d2fb9586 Mon Sep 17 00:00:00 2001 From: Zhixiang Zhan Date: Fri, 15 May 2020 00:32:54 +0800 Subject: [PATCH] fix: can not open a bot if manifest url is invalid (#3050) * passing server error to client * pretty error msg Co-authored-by: Dong Lei Co-authored-by: Chris Whitten --- .../client/src/pages/notifications/types.ts | 8 +++++++ .../pages/notifications/useNotifications.tsx | 9 +++++--- Composer/packages/client/src/store/index.tsx | 1 + .../client/src/store/reducer/index.ts | 3 ++- Composer/packages/client/src/store/types.ts | 13 ++++++++++- .../server/src/models/bot/botProject.ts | 17 +++++++++----- .../server/src/models/bot/skillManager.ts | 22 ++++++++++++++----- 7 files changed, 57 insertions(+), 16 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/types.ts b/Composer/packages/client/src/pages/notifications/types.ts index 5750bd42c1..702f69c8c0 100644 --- a/Composer/packages/client/src/pages/notifications/types.ts +++ b/Composer/packages/client/src/pages/notifications/types.ts @@ -47,6 +47,14 @@ export abstract class Notification implements INotification { } } +export class ServerNotification extends Notification { + type = NotificationType.GENERAL; + constructor(projectId: string, id: string, location: string, diagnostic: Diagnostic) { + super(projectId, id, location, diagnostic); + this.message = diagnostic.message; + } +} + export class DialogNotification extends Notification { type = NotificationType.DIALOG; constructor(projectId: string, id: string, location: string, diagnostic: Diagnostic) { diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index dbf2b6c467..665ef98cb3 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -5,13 +5,16 @@ import { useContext, useMemo } from 'react'; import { StoreContext } from '../../store'; -import { Notification, DialogNotification, LuNotification, LgNotification } from './types'; +import { Notification, DialogNotification, LuNotification, LgNotification, ServerNotification } from './types'; import { getReferredFiles } from './../../utils/luUtil'; export default function useNotifications(filter?: string) { const { state } = useContext(StoreContext); - const { dialogs, luFiles, lgFiles, projectId } = state; + const { dialogs, luFiles, lgFiles, projectId, diagnostics } = state; const memoized = useMemo(() => { const notifactions: Notification[] = []; + diagnostics.forEach(d => { + notifactions.push(new ServerNotification(projectId, '', d.source, d)); + }); dialogs.forEach(dialog => { dialog.diagnostics.map(diagnostic => { const location = `${dialog.id}.dialog`; @@ -31,7 +34,7 @@ export default function useNotifications(filter?: string) { }); }); return notifactions; - }, [dialogs, luFiles, lgFiles, projectId]); + }, [dialogs, luFiles, lgFiles, projectId, diagnostics]); const notifications: Notification[] = filter ? memoized.filter(x => x.severity === filter) : memoized; return notifications; diff --git a/Composer/packages/client/src/store/index.tsx b/Composer/packages/client/src/store/index.tsx index 306d92f239..b6369a53ec 100644 --- a/Composer/packages/client/src/store/index.tsx +++ b/Composer/packages/client/src/store/index.tsx @@ -62,6 +62,7 @@ export const initialState: State = { location: '', // the path to the bot project botEnvironment: 'production', locale: 'en-us', + diagnostics: [], botEndpoints: {}, remoteEndpoints: {}, focusPath: '', // the data path for PropertyEditor diff --git a/Composer/packages/client/src/store/reducer/index.ts b/Composer/packages/client/src/store/reducer/index.ts index d3e7c72248..ced8ba71cb 100644 --- a/Composer/packages/client/src/store/reducer/index.ts +++ b/Composer/packages/client/src/store/reducer/index.ts @@ -81,7 +81,7 @@ const initLuFilesStatus = (botName: string, luFiles: LuFile[], dialogs: DialogIn }; const getProjectSuccess: ReducerFunc = (state, { response }) => { - const { files, botName, botEnvironment, location, schemas, settings, id, locale } = response.data; + const { files, botName, botEnvironment, location, schemas, settings, id, locale, diagnostics } = response.data; schemas.sdk.content = processSchema(id, schemas.sdk.content); const { dialogs, luFiles, lgFiles, skillManifestFiles } = indexer.index(files, botName, schemas.sdk.content, locale); state.projectId = id; @@ -96,6 +96,7 @@ const getProjectSuccess: ReducerFunc = (state, { response }) => { state.luFiles = initLuFilesStatus(botName, luFiles, dialogs); state.settings = settings; state.locale = locale; + state.diagnostics = diagnostics; state.skillManifests = skillManifestFiles; refreshLocalStorage(botName, state.settings); mergeLocalStorage(botName, state.settings); diff --git a/Composer/packages/client/src/store/types.ts b/Composer/packages/client/src/store/types.ts index 42df5db641..4571f552d6 100644 --- a/Composer/packages/client/src/store/types.ts +++ b/Composer/packages/client/src/store/types.ts @@ -4,7 +4,17 @@ // TODO: remove this once we can expand the types /* eslint-disable @typescript-eslint/no-explicit-any */ import React from 'react'; -import { PromptTab, BotSchemas, ProjectTemplate, DialogInfo, LgFile, LuFile, Skill, UserSettings } from '@bfc/shared'; +import { + PromptTab, + BotSchemas, + ProjectTemplate, + DialogInfo, + LgFile, + LuFile, + Skill, + UserSettings, + Diagnostic, +} from '@bfc/shared'; import { JSONSchema7 } from '@bfc/extension'; import { AppUpdaterStatus, CreationFlowStatus, BotStatus } from '../constants'; @@ -84,6 +94,7 @@ export interface State { location: string; botEnvironment: string; locale: string; + diagnostics: Diagnostic[]; botEndpoints: { [key: string]: string }; remoteEndpoints: { [key: string]: string }; /** the data path for PropertyEditor */ diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index a1646caf19..79f6f3e2f7 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -6,7 +6,7 @@ import fs from 'fs'; import axios from 'axios'; import { autofixReferInDialog } from '@bfc/indexers'; -import { getNewDesigner, FileInfo, Skill } from '@bfc/shared'; +import { getNewDesigner, FileInfo, Skill, Diagnostic } from '@bfc/shared'; import { UserIdentity } from '@bfc/plugin-loader'; import { Path } from '../../utility/path'; @@ -67,6 +67,7 @@ export class BotProject { [key: string]: string; }; public skills: Skill[] = []; + public diagnostics: Diagnostic[] = []; public settingManager: ISettingManager; public settings: DialogSetting | null = null; constructor(ref: LocationRef, user?: UserIdentity) { @@ -84,6 +85,7 @@ export class BotProject { } public init = async () => { + this.diagnostics = []; // those 2 migrate methods shall be removed after a period of time await this._reformProjectStructure(); try { @@ -92,7 +94,9 @@ export class BotProject { // when re-index opened bot, file write may error } this.settings = await this.getEnvSettings('', false); - this.skills = await extractSkillManifestUrl(this.settings?.skill || []); + const { skillsParsed, diagnostics } = await extractSkillManifestUrl(this.settings?.skill || []); + this.skills = skillsParsed; + this.diagnostics.push(...diagnostics); this.files = await this._getFiles(); }; @@ -104,6 +108,7 @@ export class BotProject { location: this.dir, schemas: this.getSchemas(), skills: this.skills, + diagnostics: this.diagnostics, settings: this.settings, }; }; @@ -138,15 +143,15 @@ export class BotProject { // update skill in settings public updateSkill = async (config: Skill[]) => { const settings = await this.getEnvSettings('', false); - const skills = await extractSkillManifestUrl(config); + const { skillsParsed } = await extractSkillManifestUrl(config); - settings.skill = skills.map(({ manifestUrl, name }) => { + settings.skill = skillsParsed.map(({ manifestUrl, name }) => { return { manifestUrl, name }; }); await this.settingManager.set('', settings); - this.skills = skills; - return skills; + this.skills = skillsParsed; + return skillsParsed; }; public exportToZip = cb => { diff --git a/Composer/packages/server/src/models/bot/skillManager.ts b/Composer/packages/server/src/models/bot/skillManager.ts index c0a11128c4..1d61886020 100644 --- a/Composer/packages/server/src/models/bot/skillManager.ts +++ b/Composer/packages/server/src/models/bot/skillManager.ts @@ -3,7 +3,7 @@ import get from 'lodash/get'; import * as msRest from '@azure/ms-rest-js'; -import { Skill } from '@bfc/shared'; +import { Skill, Diagnostic, DiagnosticSeverity } from '@bfc/shared'; import logger from './../../logger'; @@ -46,12 +46,24 @@ export const getSkillByUrl = async (url: string, name?: string): Promise } }; -export const extractSkillManifestUrl = async (skills: any[]): Promise => { +export const extractSkillManifestUrl = async ( + skills: any[] +): Promise<{ skillsParsed: Skill[]; diagnostics: Diagnostic[] }> => { const skillsParsed: Skill[] = []; + const diagnostics: Diagnostic[] = []; for (const skill of skills) { const { manifestUrl, name } = skill; - const parsedSkill = await getSkillByUrl(manifestUrl, name); - skillsParsed.push(parsedSkill); + try { + const parsedSkill = await getSkillByUrl(manifestUrl, name); + skillsParsed.push(parsedSkill); + } catch (error) { + const notify = new Diagnostic( + `Accessing skill manifest url error, ${manifestUrl}`, + 'appsettings.json', + DiagnosticSeverity.Error + ); + diagnostics.push(notify); + } } - return skillsParsed; + return { skillsParsed, diagnostics }; };