Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UI crush on failed request status check #8575

Merged
merged 8 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Requests page crush with `Cannot read property 'target' of undefined` error
(<https://github.com/cvat-ai/cvat/pull/8575>)
2 changes: 1 addition & 1 deletion cvat-core/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
10 changes: 9 additions & 1 deletion cvat-core/src/core-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
52 changes: 40 additions & 12 deletions cvat-core/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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;
}
}
17 changes: 9 additions & 8 deletions cvat-core/src/requests-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
46 changes: 31 additions & 15 deletions cvat-core/src/server-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
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';
Expand Down Expand Up @@ -244,7 +244,7 @@
return new ServerError(message, 0);
}

function prepareData(details) {

Check warning on line 247 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
const data = new FormData();
for (const [key, value] of Object.entries(details)) {
if (Array.isArray(value)) {
Expand Down Expand Up @@ -286,7 +286,7 @@
return requestId++;
}

async function get(url: string, requestConfig) {

Check warning on line 289 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
return new Promise((resolve, reject) => {
const newRequestId = getRequestId();
requests[newRequestId] = { resolve, reject };
Expand Down Expand Up @@ -819,7 +819,7 @@
save_images: saveImages,
};
return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 822 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
Axios.post(baseURL, {}, {
params,
})
Expand Down Expand Up @@ -914,7 +914,7 @@
const url = `${backendAPI}/tasks/${id}/backup/export`;

return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 917 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
try {
const response = await Axios.post(url, {}, {
params,
Expand Down Expand Up @@ -996,7 +996,7 @@
const url = `${backendAPI}/projects/${id}/backup/export`;

return new Promise<string | void>((resolve, reject) => {
async function request() {

Check warning on line 999 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
try {
const response = await Axios.post(url, {}, {
params,
Expand Down Expand Up @@ -1069,7 +1069,7 @@
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
Expand Down Expand Up @@ -1104,11 +1104,11 @@

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, {
Expand All @@ -1118,13 +1118,13 @@
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) {

Check warning on line 1127 in cvat-core/src/server-proxy.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
const fileBulks = files.reduce((fileGroups, file) => {
const lastBulk = fileGroups[fileGroups.length - 1];
if (chunkSize - lastBulk.size >= file.size) {
Expand All @@ -1142,11 +1142,11 @@
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 },
Expand All @@ -1170,11 +1170,11 @@
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,
Expand Down Expand Up @@ -2250,16 +2250,32 @@
}
}

// 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> {
Expand Down
13 changes: 7 additions & 6 deletions cvat-core/src/server-response-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cvat-core/src/session-implementation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion cvat-core/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 13 additions & 4 deletions cvat-ui/src/actions/requests-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 {
klakhov marked this conversation as resolved.
Show resolved Hide resolved
return (
typeof rqID === 'string' &&
(!state.requests[rqID] || [RQStatus.FINISHED, RQStatus.FAILED].includes(state.requests[rqID]?.status))
Expand All @@ -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],
});
}
Loading
Loading