From e6bac3a72ab213a6e288a8af58a74c3b0c6eae5c Mon Sep 17 00:00:00 2001 From: EMaslowskiQ <118929649+EMaslowskiQ@users.noreply.github.com> Date: Wed, 14 Aug 2024 16:58:12 -0400 Subject: [PATCH] DPO3DPKRT-843/invalid Scenes Crash Batch Download (#620) (fix) delay batch submissions to avoid tight loop (fix) invalid scenes crashing batch download process (fix) error responses not accurate or clear when only some jobs failed (fix) cleanup logs and comments --- client/src/api/index.ts | 5 +- .../pages/Admin/components/AdminToolsView.tsx | 44 ++++-- server/http/routes/api/generateDownloads.ts | 126 ++++-------------- .../workflow/impl/Packrat/WorkflowEngine.ts | 2 +- 4 files changed, 69 insertions(+), 108 deletions(-) diff --git a/client/src/api/index.ts b/client/src/api/index.ts index e4fe2c00..8eb03c99 100644 --- a/client/src/api/index.ts +++ b/client/src/api/index.ts @@ -46,7 +46,6 @@ export default class API { const body = JSON.stringify({ statusOnly, rePublish, idSystemObject }); let uri: string = API_ROUTES.GEN_DOWNLOADS; console.log('[PACKRAT:DEBUG] body: ',body); - console.trace('API.generateDownloads'); let options; if(statusOnly) { @@ -82,8 +81,10 @@ export default class API { return fetch(`${serverEndpoint}/${route}`, defaultOptions) .then(response => { // Check if the response returned a successful status code - if (!response.ok) + if (!response.ok) { + console.log('response: ',response); return { success: false, message: response.statusText }; + } return response.json(); // Assuming the server responds with JSON }) .catch(error => { diff --git a/client/src/pages/Admin/components/AdminToolsView.tsx b/client/src/pages/Admin/components/AdminToolsView.tsx index 3a0aea40..17095c5f 100644 --- a/client/src/pages/Admin/components/AdminToolsView.tsx +++ b/client/src/pages/Admin/components/AdminToolsView.tsx @@ -636,15 +636,43 @@ const AdminToolsBatchGeneration = (): React.ReactElement => { const response: RequestResponse = await API.generateDownloads(sceneIDs,false,republishScenes); if(response.success === false) { - // if the job is running then handle differently - if(response.message && response.message.includes('already running')) { - console.log(`[Packrat:WARN] cannot do ${BatchOperations[operation]}. (${response.message})`); - toast.warn(`Not running ${BatchOperations[operation]}. Job already running. Please wait for it to finish.`); - } else { - console.log(`[Packrat:ERROR] cannot run ${BatchOperations[operation]}. (${response.message})`); - toast.error(`Cannot ${BatchOperations[operation]}. Check the report.`); + // make sure we have data and responses + if(!response.data || !Array.isArray(response.data)) { + console.log(`[Packrat:ERROR] cannot run ${BatchOperations[operation]}. invalid response data.`,response); + toast.error(`${BatchOperations[operation]} failed. Got unexpected data from server.`); + return; } - return; + + // get our unique error messages + const uniqueMessages = Array.from( + new Set( + response.data + .filter(response => !response.success && response.message) // Ensure there is a message + .map(response => `${response.id}: ${response.message}`) // Extract the messages + ) + ); + const toastErrorMsg: string = (uniqueMessages.length>1) ? 'Check the console.' : uniqueMessages[0]; + + // see if we have nuance to the response (i.e. some failed/some passed) + const allFailed: boolean = response.data.every( response => response.succcess===false ); + if(allFailed===true) { + const errorMsg: string = (response.data.length>1) + ? `All ${response.data.length} scenes failed during ${BatchOperations[operation]} run.` + : `${BatchOperations[operation]} cannot run. ${uniqueMessages[0]}`; + + console.log(`[Packrat:ERROR] ${errorMsg}`,response.data); + toast.error(`${BatchOperations[operation]} failed. (${toastErrorMsg})`); + return; + } + + // only some failed so we need to handle this + const failedCount: number = response.data.filter(response => !response.success).length; + console.log(`[Packrat:ERROR] ${response.data.length}/${selectedList.length} scenes failed. (${uniqueMessages.join(' |')})`,response.data); + toast.warn(`${BatchOperations[operation]} had issues. ${failedCount} scenes failed. (${toastErrorMsg})`); + + // we bail early so the selection is maintained on failure + // TODO: deselect those that were successful. + return false; } // clear selection on succcess diff --git a/server/http/routes/api/generateDownloads.ts b/server/http/routes/api/generateDownloads.ts index d777fed5..7f81e2f9 100644 --- a/server/http/routes/api/generateDownloads.ts +++ b/server/http/routes/api/generateDownloads.ts @@ -42,16 +42,31 @@ const generateResponse = (success: boolean, message?: string | undefined, id?: n state }; }; -const buildOpResponse = (message: string, responses: GenDownloadsResponse[]): OpResponse => { - // if empty send error response - if(responses.length===0) - return { success: false, message: 'no responses from operation' }; +const buildOpResponse = (responses: GenDownloadsResponse[]): OpResponse => { + if (responses.length === 0) { + return { success: false, message: 'No responses from operation', data: [] }; + } + + // see how many were successfuly and extract error messages from those not successful + const successCount = responses.filter(response => response.success).length; + const messages = Array.from( + new Set( + responses + .filter(response => !response.success && response.message) + .map(response => response.message as string) + ) + ); + + // build up our final message based on what was found + const message = + successCount === responses.length + ? `All ${successCount} scenes submitted successfully` + : `${successCount}/${responses.length} scenes submitted successfully (errors: ${messages.join(', ')})`; - // cycle through all responses and see if we return { - success: responses.every((r)=>r.success), + success: successCount === responses.length, message, - data: [...responses], + data: responses, }; }; @@ -77,10 +92,10 @@ const createOpForScene = async (idSystemObject: number, idUser: number): Promise // create our workflow for generating downloads const result: WorkflowCreateResult = await wfEngine.generateDownloads(scene.idScene, workflowParams); - // LOG.info(`API.generateDownloads post creation. (result: ${H.Helpers.JSONStringify(result)})`,LOG.LS.eDEBUG); + LOG.info(`API.generateDownloads post creation. (result: ${H.Helpers.JSONStringify(result)})`,LOG.LS.eDEBUG); const isValid: boolean = result.data.isValid ?? false; - const isJobRunning: boolean = (result.data.activeJobs.length>0) ?? false; + const isJobRunning: boolean = (result.data.activeJobs?.length>0) ?? false; const idWorkflow: number | undefined = (result.data.workflow?.idWorkflow) ?? undefined; const idWorkflowReport: number | undefined = (result.data.workflowReport?.idWorkflowReport) ?? undefined; @@ -90,6 +105,7 @@ const createOpForScene = async (idSystemObject: number, idUser: number): Promise return generateResponse(false,result.message,idSystemObject,{ isValid, isJobRunning, idWorkflow, idWorkflowReport }); } + console.log('a'); return generateResponse(true,`Generating Downloads for: ${scene.Name}`,idSystemObject,{ isValid, isJobRunning, idWorkflow, idWorkflowReport }); }; const getOpStatusForScene = async (idSystemObject: number): Promise => { @@ -241,79 +257,6 @@ const publishScene = async (response: GenDownloadsResponse, intendedState: COMMO return; }; -// queue management -// enum OpStatus { -// UNDEFINED, -// PENDING, -// RUNNING, -// FINISHED, -// ERROR, -// RETRY -// } -// type OpsQueueItem = { -// status: OpStatus; -// idSystemObject: number; -// message?: string; -// response: Promise | null; -// }; -// const processScenes = async (idSystemObjects: number[], idUser: number): Promise => { -// const maxActive: number = 3; - -// // add our ids to the queue -// const queue: OpsQueueItem[] = idSystemObjects.map((id) => ({ -// status: OpStatus.PENDING, -// idSystemObject: id, -// response: null, -// })); - -// // hold our active items and any returned promises -// const active: Set = new Set(); -// const responses: GenDownloadsResponse[] = []; - -// // function to process the next item checking that we're not exceeding our -// // concurrency and checking status of all items -// const processNext = async () => { -// // if over our max active, just return (nothing to add) -// if (active.size >= maxActive) return; - -// // find the next item based on the status. find is sequential so we get a FIFO behavior -// const nextItem: OpsQueueItem | undefined = queue.find(item => item.status === OpStatus.PENDING || item.status === OpStatus.RETRY); -// if (!nextItem) return; - -// // update our status and add to list of active items -// nextItem.status = OpStatus.RUNNING; -// active.add(nextItem); - -// try { -// // we create the op and wait for it to finish -// // TODO: returns FINISH before checking state of the workflow -// nextItem.response = createOpForScene(nextItem.idSystemObject, idUser); -// const response = await nextItem.response; -// responses.push(response); -// nextItem.status = OpStatus.FINISHED; -// } catch (error) { -// // set our error and push a failed attempt into our responses -// nextItem.status = OpStatus.ERROR; -// nextItem.message = error as string; -// responses.push(generateResponse(false,error as string,nextItem.idSystemObject)); -// } finally { -// // remove from our active items and process a new one -// active.delete(nextItem); -// processNext(); -// } -// }; - -// // cycle through all queue items constantly -// while (queue.some(item => item.status !== OpStatus.FINISHED && item.status !== OpStatus.ERROR)) { -// if (active.size < maxActive) { -// processNext(); -// } -// await new Promise(resolve => setTimeout(resolve, 100)); // Small delay to prevent tight loop -// } - -// return responses; -// }; - export async function generateDownloads(req: Request, res: Response): Promise { // make sure we're authenticated (i.e. see if request has a 'user' object) @@ -376,16 +319,13 @@ export async function generateDownloads(req: Request, res: Response): PromisemaxIDs) { LOG.info('API.generateDownloads too many scenes submitted. limiting to 10',LOG.LS.eHTTP); - messageSuffix = `(Capped to ${maxIDs})`; idSystemObjects.splice(10); } // cycle through IDs - let messagePrefix: string = ''; const responses: GenDownloadsResponse[] = []; for(let i=0; i