Fix UI crush on failed request status check (#8575)

### Motivation and context  When a status check for `request` is failing for any reason, eg something happened on the server and the request failed with 50X error. The requests page crushes with an error: `Cannot read property 'target' of undefined` : ![image](https://github.com/user-attachments/assets/e41ea391-c03e-47cd-9ab0-1e06583d366c) Now the error message is shown: ![image](https://github.com/user-attachments/assets/e9976657-2fc8-4be8-94b4-1d5453b9d9ab) ### How has this been tested?  ### Checklist  - [x] I submit my changes into the `develop` branch - [x] I have created a changelog fragment  - ~~[ ] I have updated the documentation accordingly~~ - [ ] I have added tests to cover my changes - ~~[ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))~~ - [x] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern.  ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced request handling with improved initialization and error management for tasks and jobs. - Added `listenToCreate` method in the Task class for better task creation management. - Introduced `generateInitialRequest` function to standardize request initialization across imports and exports. - **Bug Fixes** - Improved clarity of status messages for failed requests in the RequestCard component. - **Documentation** - Updated method signatures across various action files for better clarity and consistency. - **Chores** - Refined import statements and type exports in the core wrapper to streamline functionality.
cvat-ai · Oct 29, 2024 · e27f93e · e27f93e
1 parent 37586c0
commit e27f93e
Show file tree

Hide file tree

Showing 14 changed files with 137 additions and 60 deletions.
diff --git a/changelog.d/20241022_121638_klakhov_fix_request_status_crush.md b/changelog.d/20241022_121638_klakhov_fix_request_status_crush.md
@@ -0,0 +1,4 @@
+### Fixed
+
+- Requests page crush with `Cannot read property 'target' of undefined` error
+  (<https://github.com/cvat-ai/cvat/pull/8575>)
diff --git a/cvat-core/package.json b/cvat-core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "cvat-core",
-  "version": "15.2.0",
+  "version": "15.2.1",
   "type": "module",
   "description": "Part of Computer Vision Tool which presents an interface for client-side integration",
   "main": "src/api.ts",

diff --git a/cvat-core/src/core-types.ts b/cvat-core/src/core-types.ts
@@ -2,7 +2,9 @@
 //
 // SPDX-License-Identifier: MIT
 
-import { ModelKind, ModelReturnType, ShapeType } from './enums';
+import {
+    ModelKind, ModelReturnType, RQStatus, ShapeType,
+} from './enums';
 
 export interface ModelAttribute {
     name: string;
@@ -54,4 +56,10 @@ export interface SerializedModel {
     updated_date?: string;
 }
 
+export interface UpdateStatusData {
+    status: RQStatus;
+    progress: number;
+    message: string;
+}
+
 export type PaginatedResource<T> = T[] & { count: number };
diff --git a/cvat-core/src/request.ts b/cvat-core/src/request.ts
@@ -6,10 +6,10 @@ import { RQStatus } from './enums';
 import User from './user';
 import { SerializedRequest } from './server-response-types';
 
-type Operation = {
+export type RequestOperation = {
     target: string;
     type: string;
-    format: string;
+    format: string | null;
     jobID: number | null;
     taskID: number | null;
     projectID: number | null;
@@ -44,9 +44,7 @@ export class Request {
         this.#finishedDate = initialData.finished_date;
         this.#expiryDate = initialData.expiry_date;
 
-        if (initialData.owner) {
-            this.#owner = new User(initialData.owner);
-        }
+        this.#owner = new User(initialData.owner);
     }
 
     get id(): string {
@@ -57,15 +55,15 @@ export class Request {
         return this.#status.toLowerCase() as RQStatus;
     }
 
-    get progress(): number {
+    get progress(): number | undefined {
         return this.#progress;
     }
 
     get message(): string {
         return this.#message;
     }
 
-    get operation(): Operation {
+    get operation(): RequestOperation {
         return {
             target: this.#operation.target,
             type: this.#operation.type,
@@ -77,31 +75,61 @@ export class Request {
         };
     }
 
-    get url(): string {
+    get url(): string | undefined {
         return this.#resultUrl;
     }
 
-    get resultID(): number {
+    get resultID(): number | undefined {
         return this.#resultID;
     }
 
     get createdDate(): string {
         return this.#createdDate;
     }
 
-    get startedDate(): string {
+    get startedDate(): string | undefined {
         return this.#startedDate;
     }
 
-    get finishedDate(): string {
+    get finishedDate(): string | undefined {
         return this.#finishedDate;
     }
 
-    get expiryDate(): string {
+    get expiryDate(): string | undefined {
         return this.#expiryDate;
     }
 
     get owner(): User {
         return this.#owner;
     }
+
+    public toJSON(): SerializedRequest {
+        const result: SerializedRequest = {
+            id: this.#id,
+            status: this.#status,
+            operation: {
+                target: this.#operation.target,
+                type: this.#operation.type,
+                format: this.#operation.format,
+                job_id: this.#operation.job_id,
+                task_id: this.#operation.task_id,
+                project_id: this.#operation.project_id,
+                function_id: this.#operation.function_id,
+            },
+            progress: this.#progress,
+            message: this.#message,
+            result_url: this.#resultUrl,
+            result_id: this.#resultID,
+            created_date: this.#createdDate,
+            started_date: this.#startedDate,
+            finished_date: this.#finishedDate,
+            expiry_date: this.#expiryDate,
+            owner: {
+                id: this.#owner.id,
+                username: this.#owner.username,
+            },
+        };
+
+        return result;
+    }
 }
diff --git a/cvat-core/src/requests-manager.ts b/cvat-core/src/requests-manager.ts
@@ -122,14 +122,15 @@ class RequestsManager {
                     }
                 } catch (error) {
                     if (requestID in this.listening) {
-                        const { onUpdate } = this.listening[requestID];
-
-                        onUpdate
-                            .forEach((update) => update(new Request({
-                                id: requestID,
-                                status: RQStatus.FAILED,
-                                message: `Could not get a status of the request ${requestID}. ${error.toString()}`,
-                            })));
+                        const { onUpdate, request } = this.listening[requestID];
+                        if (request) {
+                            onUpdate
+                                .forEach((update) => update(new Request({
+                                    ...request.toJSON(),
+                                    status: RQStatus.FAILED,
+                                    message: `Could not get a status of the request ${requestID}. ${error.toString()}`,
+                                })));
+                        }
                         reject(error);
                     }
                 }

diff --git a/cvat-core/src/server-proxy.ts b/cvat-core/src/server-proxy.ts
@@ -21,7 +21,7 @@ import {
     SerializedQualityReportData, APIQualityReportsFilter, SerializedAnalyticsReport, APIAnalyticsReportFilter,
     SerializedRequest, SerializedJobValidationLayout, SerializedTaskValidationLayout,
 } from './server-response-types';
-import { PaginatedResource } from './core-types';
+import { PaginatedResource, UpdateStatusData } from './core-types';
 import { Request } from './request';
 import { Storage } from './storage';
 import { SerializedEvent } from './event';
@@ -1069,7 +1069,7 @@ type LongProcessListener<R> = Record<number, {
 async function createTask(
     taskSpec: Partial<SerializedTask>,
     taskDataSpec: any,
-    onUpdate: (request: Request) => void,
+    onUpdate: (request: Request | UpdateStatusData) => void,
 ): Promise<{ taskID: number, rqID: string }> {
     const { backendAPI, origin } = config;
     // keep current default params to 'freeze" them during this request
@@ -1104,11 +1104,11 @@ async function createTask(
 
     let response = null;
 
-    onUpdate(new Request({
+    onUpdate({
         status: RQStatus.UNKNOWN,
         progress: 0,
         message: 'CVAT is creating your task',
-    }));
+    });
 
     try {
         response = await Axios.post(`${backendAPI}/tasks`, taskSpec, {
@@ -1118,11 +1118,11 @@ async function createTask(
         throw generateError(errorData);
     }
 
-    onUpdate(new Request({
+    onUpdate({
         status: RQStatus.UNKNOWN,
         progress: 0,
         message: 'CVAT is uploading task data to the server',
-    }));
+    });
 
     async function bulkUpload(taskId, files) {
         const fileBulks = files.reduce((fileGroups, file) => {
@@ -1142,11 +1142,11 @@ async function createTask(
                 taskData.append(`client_files[${idx}]`, element);
             }
             const percentage = totalSentSize / totalSize;
-            onUpdate(new Request({
+            onUpdate({
                 status: RQStatus.UNKNOWN,
                 progress: percentage,
                 message: 'CVAT is uploading task data to the server',
-            }));
+            });
             await Axios.post(`${backendAPI}/tasks/${taskId}/data`, taskData, {
                 ...params,
                 headers: { 'Upload-Multiple': true },
@@ -1170,11 +1170,11 @@ async function createTask(
         const uploadConfig = {
             endpoint: `${origin}${backendAPI}/tasks/${response.data.id}/data/`,
             onUpdate: (percentage) => {
-                onUpdate(new Request({
+                onUpdate({
                     status: RQStatus.UNKNOWN,
                     progress: percentage,
                     message: 'CVAT is uploading task data to the server',
-                }));
+                });
             },
             chunkSize,
             totalSize,
@@ -2250,16 +2250,32 @@ async function getRequestsList(): Promise<PaginatedResource<SerializedRequest>>
     }
 }
 
+// Temporary solution for server availability problems
+const retryTimeouts = [5000, 10000, 15000];
 async function getRequestStatus(rqID: string): Promise<SerializedRequest> {
     const { backendAPI } = config;
+    let retryCount = 0;
+    let lastError = null;
 
-    try {
-        const response = await Axios.get(`${backendAPI}/requests/${rqID}`);
+    while (retryCount < 3) {
+        try {
+            const response = await Axios.get(`${backendAPI}/requests/${rqID}`);
 
-        return response.data;
-    } catch (errorData) {
-        throw generateError(errorData);
+            return response.data;
+        } catch (errorData) {
+            lastError = generateError(errorData);
+            const { response } = errorData;
+            if (response && [502, 503, 504].includes(response.status)) {
+                const timeout = retryTimeouts[retryCount];
+                await new Promise((resolve) => { setTimeout(resolve, timeout); });
+                retryCount++;
+            } else {
+                throw generateError(errorData);
+            }
+        }
     }
+
+    throw lastError;
 }
 
 async function cancelRequest(requestID): Promise<void> {

diff --git a/cvat-core/src/server-response-types.ts b/cvat-core/src/server-response-types.ts
@@ -504,25 +504,26 @@ export interface SerializedAPISchema {
 }
 
 export interface SerializedRequest {
-    id?: string;
+    id: string;
+    message: string;
     status: string;
-    operation?: {
+    operation: {
         target: string;
         type: string;
-        format: string;
+        format: string | null;
         job_id: number | null;
         task_id: number | null;
         project_id: number | null;
+        function_id: string | null;
     };
     progress?: number;
-    message: string;
     result_url?: string;
     result_id?: number;
-    created_date?: string;
+    created_date: string;
     started_date?: string;
     finished_date?: string;
     expiry_date?: string;
-    owner?: any;
+    owner: any;
 }
 
 export interface SerializedJobValidationLayout {

diff --git a/cvat-core/src/session-implementation.ts b/cvat-core/src/session-implementation.ts
@@ -756,12 +756,12 @@ export function implementTask(Task: typeof TaskClass): typeof TaskClass {
             const { taskID, rqID } = await serverProxy.tasks.create(
                 taskSpec,
                 taskDataSpec,
-                options?.requestStatusCallback || (() => {}),
+                options?.updateStatusCallback || (() => {}),
             );
 
             await requestsManager.listen(rqID, {
                 callback: (request: Request) => {
-                    options?.requestStatusCallback(request);
+                    options?.updateStatusCallback(request);
                     if (request.status === RQStatus.FAILED) {
                         serverProxy.tasks.delete(taskID, config.organization.organizationSlug || null);
                     }

diff --git a/cvat-core/src/session.ts b/cvat-core/src/session.ts
@@ -29,6 +29,7 @@ import logger from './logger';
 import Issue from './issue';
 import ObjectState from './object-state';
 import { JobValidationLayout, TaskValidationLayout } from './validation-layout';
+import { UpdateStatusData } from './core-types';
 
 function buildDuplicatedAPI(prototype) {
     Object.defineProperties(prototype, {
@@ -1141,7 +1142,7 @@ export class Task extends Session {
 
     async save(
         fields: Record<string, any> = {},
-        options?: { requestStatusCallback?: (request: Request) => void },
+        options?: { updateStatusCallback?: (updateData: Request | UpdateStatusData) => void },
     ): Promise<Task> {
         const result = await PluginRegistry.apiWrapper.call(this, Task.prototype.save, fields, options);
         return result;

diff --git a/cvat-ui/src/actions/requests-actions.ts b/cvat-ui/src/actions/requests-actions.ts
@@ -3,12 +3,21 @@
 // SPDX-License-Identifier: MIT
 
 import { ActionUnion, createAction } from 'utils/redux';
-import { RequestsQuery, RequestsState } from 'reducers';
+import { CombinedState, RequestsQuery, RequestsState } from 'reducers';
 import {
     Request, ProjectOrTaskOrJob, getCore, RQStatus,
 } from 'cvat-core-wrapper';
+import { Store } from 'redux';
+import { getCVATStore } from 'cvat-store';
 
 const core = getCore();
+let store: null | Store<CombinedState> = null;
+function getStore(): Store<CombinedState> {
+    if (store === null) {
+        store = getCVATStore();
+    }
+    return store;
+}
 
 export enum RequestsActionsTypes {
     GET_REQUESTS = 'GET_REQUESTS',
@@ -79,7 +88,7 @@ export function updateRequestProgress(request: Request, dispatch: (action: Reque
     );
 }
 
-export function shouldListenForProgress(rqID: string | undefined, state: RequestsState): boolean {
+export function shouldListenForProgress(rqID: string | void, state: RequestsState): boolean {
     return (
         typeof rqID === 'string' &&
         (!state.requests[rqID] || [RQStatus.FINISHED, RQStatus.FAILED].includes(state.requests[rqID]?.status))
@@ -89,13 +98,13 @@ export function shouldListenForProgress(rqID: string | undefined, state: Request
 export function listen(
     requestID: string,
     dispatch: (action: RequestsActions) => void,
-    initialRequest?: Request,
 ) : Promise<Request> {
+    const { requests } = getStore().getState().requests;
     return core.requests
         .listen(requestID, {
             callback: (updatedRequest) => {
                 updateRequestProgress(updatedRequest, dispatch);
             },
-            initialRequest,
+            initialRequest: requests[requestID],
         });
 }