20219: Pass through promise rejections from getLogFileContents
authorStephen Smith <stephen@curii.com>
Wed, 2 Aug 2023 21:48:36 +0000 (17:48 -0400)
committerStephen Smith <stephen@curii.com>
Wed, 2 Aug 2023 21:48:36 +0000 (17:48 -0400)
* No longer returns undefined for failed log fragments
* initProcessLogsPanel added catch to handle errors, on failure it shows a
  toast and initializes the store empty, allowing polling to run and retry if
  the container is running
* Add note to pollProcessLogs error console.log that the promise rejection is
  ignored currently
* loadContainerLogFileContents now handles errors from getLogFileContents
  * Switched from Promise.all to Promise.allSettled, which allows some promises
    to fail and still return a result
  * Replace undefined fragment & subfragment filtering with top level promise
    status filtering, subfragments can no longer have some failures due to
    sub-promises using .all which rejects on any failure
* Add es2020 to typescript lib array for Promise.allSettled

Arvados-DCO-1.1-Signed-off-by: Stephen Smith <stephen@curii.com>

src/services/log-service/log-service.ts
src/store/process-logs-panel/process-logs-panel-actions.ts
tsconfig.json

index 4ba02befc336519d17bc7584cd70b6432fcde160..03d3c01ec446c2a3374b3fafeca40a0a682aeefd 100644 (file)
@@ -31,24 +31,20 @@ export class LogService extends CommonResourceService<LogResource> {
         return Promise.reject();
     }
 
-    async getLogFileContents(containerRequestUuid: string, fileRecord: CollectionFile, startByte: number, endByte: number): Promise<LogFragment | undefined> {
-        try {
-            const request = await this.apiWebdavClient.get(
-                `container_requests/${containerRequestUuid}/log/${fileRecord.name}`,
-                {headers: {Range: `bytes=${startByte}-${endByte}`}}
-            );
-            const logFileType = logFileToLogType(fileRecord);
+    async getLogFileContents(containerRequestUuid: string, fileRecord: CollectionFile, startByte: number, endByte: number): Promise<LogFragment> {
+        const request = await this.apiWebdavClient.get(
+            `container_requests/${containerRequestUuid}/log/${fileRecord.name}`,
+            {headers: {Range: `bytes=${startByte}-${endByte}`}}
+        );
+        const logFileType = logFileToLogType(fileRecord);
 
-            if (request.responseText && logFileType) {
-                return {
-                    logType: logFileType,
-                    contents: request.responseText.split(/\r?\n/),
-                };
-            } else {
-                return undefined;
-            }
-        } catch(e) {
-            return undefined;
+        if (request.responseText && logFileType) {
+            return {
+                logType: logFileType,
+                contents: request.responseText.split(/\r?\n/),
+            };
+        } else {
+            return Promise.reject();
         }
     }
 }
index af30e2707fdd0b4b0373f04e454679f8f869e106..9da7d1b9110d0f7a24dfc6700229fbf277d4ed22 100644 (file)
@@ -39,21 +39,28 @@ export const setProcessLogsPanelFilter = (filter: string) =>
 
 export const initProcessLogsPanel = (processUuid: string) =>
     async (dispatch: Dispatch, getState: () => RootState, { logService }: ServiceRepository) => {
-        dispatch(processLogsPanelActions.RESET_PROCESS_LOGS_PANEL());
-        const process = getProcess(processUuid)(getState().resources);
-        if (process?.containerRequest?.uuid) {
-            // Get log file size info
-            const logFiles = await loadContainerLogFileList(process.containerRequest.uuid, logService);
+        try {
+            dispatch(processLogsPanelActions.RESET_PROCESS_LOGS_PANEL());
+            const process = getProcess(processUuid)(getState().resources);
+            if (process?.containerRequest?.uuid) {
+                // Get log file size info
+                const logFiles = await loadContainerLogFileList(process.containerRequest.uuid, logService);
 
-            // Populate lastbyte 0 for each file
-            const filesWithProgress = logFiles.map((file) => ({file, lastByte: 0}));
+                // Populate lastbyte 0 for each file
+                const filesWithProgress = logFiles.map((file) => ({file, lastByte: 0}));
 
-            // Fetch array of LogFragments
-            const logLines = await loadContainerLogFileContents(filesWithProgress, logService, process);
+                // Fetch array of LogFragments
+                const logLines = await loadContainerLogFileContents(filesWithProgress, logService, process);
 
-            // Populate initial state with filters
-            const initialState = createInitialLogPanelState(logFiles, logLines);
+                // Populate initial state with filters
+                const initialState = createInitialLogPanelState(logFiles, logLines);
+                dispatch(processLogsPanelActions.INIT_PROCESS_LOGS_PANEL(initialState));
+            }
+        } catch(e) {
+            // On error, populate empty state to allow polling to start
+            const initialState = createInitialLogPanelState([], []);
             dispatch(processLogsPanelActions.INIT_PROCESS_LOGS_PANEL(initialState));
+            dispatch(snackbarActions.OPEN_SNACKBAR({ message: 'Could not load process logs', hideDuration: 2000, kind: SnackbarKind.ERROR }));
         }
     };
 
@@ -94,6 +101,7 @@ export const pollProcessLogs = (processUuid: string) =>
             }
             return Promise.resolve();
         } catch (e) {
+            // Remove log when polling error is handled in some way instead of being ignored
             console.log("Polling process logs failed");
             return Promise.reject();
         }
@@ -120,7 +128,7 @@ const loadContainerLogFileList = async (containerUuid: string, logService: LogSe
  * @returns LogFragment[] containing a single LogFragment corresponding to each input file
  */
 const loadContainerLogFileContents = async (logFilesWithProgress: FileWithProgress[], logService: LogService, process: Process) => (
-    (await Promise.all(logFilesWithProgress.filter(({file}) => file.size > 0).map(({file, lastByte}) => {
+    (await Promise.allSettled(logFilesWithProgress.filter(({file}) => file.size > 0).map(({file, lastByte}) => {
         const requestSize = file.size - lastByte;
         if (requestSize > maxLogFetchSize) {
             const chunkSize = Math.floor(maxLogFetchSize / 2);
@@ -128,31 +136,38 @@ const loadContainerLogFileContents = async (logFilesWithProgress: FileWithProgre
             return Promise.all([
                 logService.getLogFileContents(process.containerRequest.uuid, file, lastByte, firstChunkEnd),
                 logService.getLogFileContents(process.containerRequest.uuid, file, file.size-chunkSize, file.size-1)
-            ] as Promise<(LogFragment | undefined)>[]);
+            ] as Promise<(LogFragment)>[]);
         } else {
             return Promise.all([logService.getLogFileContents(process.containerRequest.uuid, file, lastByte, file.size-1)]);
         }
-    }))).filter((logResponseSet) => ( // Top level filter ensures array of LogFragments is not empty and contains 1 or more fragments containing log lines
-        logResponseSet.length && logResponseSet.some(logFragment => logFragment && logFragment.contents.length)
-    )).map((logResponseSet)=> {
-        // Remove fragments from subarrays that are undefined or have no lines
-        const responseSet = logResponseSet.filter((logFragment): logFragment is LogFragment => (
-            !!logFragment && logFragment.contents.length > 0
-        ));
-
+    })).then((res) => {
+        if (res.length && res.every(promise => (promise.status === 'rejected'))) {
+            // Since allSettled does not pass promise rejection we throw an
+            //   error if every request failed
+            return Promise.reject("Failed to load logs");
+        }
+        return res.filter((one): one is PromiseFulfilledResult<LogFragment[]> => (
+            // Filter out log files with rejected promises
+            //   (Promise.all rejects on any failure)
+            one.status === 'fulfilled' &&
+            // Filter out files where any fragment is empty
+            //   (prevent incorrect snipline generation or an un-resumable situation)
+            !!one.value.every(logFragment => logFragment.contents.length)
+        )).map(one => one.value)
+    })).map((logResponseSet)=> {
         // For any multi fragment response set, modify the last line of non-final chunks to include a line break and snip line
         //   Don't add snip line as a separate line so that sorting won't reorder it
-        for (let i = 1; i < responseSet.length; i++) {
-            const fragment = responseSet[i-1];
+        for (let i = 1; i < logResponseSet.length; i++) {
+            const fragment = logResponseSet[i-1];
             const lastLineIndex = fragment.contents.length-1;
             const lastLineContents = fragment.contents[lastLineIndex];
             const newLastLine = `${lastLineContents}\n${SNIPLINE}`;
 
-            responseSet[i-1].contents[lastLineIndex] = newLastLine;
+            logResponseSet[i-1].contents[lastLineIndex] = newLastLine;
         }
 
         // Merge LogFragment Array (representing multiple log line arrays) into single LogLine[] / LogFragment
-        return responseSet.reduce((acc, curr: LogFragment) => ({
+        return logResponseSet.reduce((acc, curr: LogFragment) => ({
             logType: curr.logType,
             contents: [...(acc.contents || []), ...curr.contents]
         }), {} as LogFragment);
index 7bce40227d5706bcabd95c8448d82a6679f83fdf..08f7108e6230629b7bb787ecfda07f41a51390f6 100644 (file)
@@ -5,6 +5,7 @@
     "target": "es5",
     "lib": [
       "es6",
+      "es2020",
       "dom"
     ],
     "sourceMap": true,