Skip to content

Commit 737ac09

Browse files
committed
fix(FR-1378): improve error handling logic in service update flow (#4166)
Resolves #4153 ([FR-1378](https://lablup.atlassian.net/browse/FR-1378)) ## Summary Improves error handling logic in the service update flow to properly display server error messages when endpoint modification fails. ## Changes - Removed unused legacy mutation function `legacyMutationToUpdateService` - Simplified conditional logic by removing `baiClient.supports('modify-endpoint')` check - Streamlined error handling to ensure server error messages are properly displayed - Consolidated success/error message flow for better user experience ## Test Plan - [ ] Test endpoint modification with invalid image name format to verify proper error message display - [ ] Verify successful endpoint updates still show success messages correctly - [ ] Ensure all existing functionality remains intact **Type:** Bug Fix **Component:** Service Launcher [FR-1378]: https://lablup.atlassian.net/browse/FR-1378?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent c89cbd3 commit 737ac09

File tree

1 file changed

+76
-107
lines changed

1 file changed

+76
-107
lines changed

react/src/components/ServiceLauncherPageContent.tsx

Lines changed: 76 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -274,20 +274,6 @@ const ServiceLauncherPageContent: React.FC<ServiceLauncherPageContentProps> = ({
274274
};
275275
};
276276

277-
const legacyMutationToUpdateService = useTanMutation({
278-
mutationFn: (values: ServiceLauncherFormValue) => {
279-
const body = {
280-
to: values.replicas,
281-
};
282-
return baiSignedRequestWithPromise({
283-
method: 'POST',
284-
url: `/services/${endpoint?.endpoint_id}/scale`,
285-
body,
286-
client: baiClient,
287-
});
288-
},
289-
});
290-
291277
const mutationToCreateService = useTanMutation<
292278
unknown,
293279
| {
@@ -492,108 +478,91 @@ const ServiceLauncherPageContent: React.FC<ServiceLauncherPageContentProps> = ({
492478
.validateFields()
493479
.then((values) => {
494480
if (endpoint) {
495-
if (baiClient.supports('modify-endpoint')) {
496-
const mutationVariables: ServiceLauncherPageContentModifyMutation['variables'] =
497-
{
498-
endpoint_id: endpoint?.endpoint_id || '',
499-
props: {
500-
resource_slots: JSON.stringify({
501-
cpu: values.resource.cpu,
502-
mem: values.resource.mem,
503-
...(values.resource.accelerator > 0
504-
? {
505-
[values.resource.acceleratorType]:
506-
values.resource.accelerator,
507-
}
508-
: undefined),
509-
}),
510-
resource_opts: JSON.stringify({
511-
shmem: values.resource.shmem,
512-
}),
513-
// FIXME: temporarily convert cluster mode string according to server-side type
514-
cluster_mode:
515-
'single-node' === values.cluster_mode
516-
? 'SINGLE_NODE'
517-
: 'MULTI_NODE',
518-
cluster_size: values.cluster_size,
519-
...(baiClient.supports('replicas')
520-
? { replicas: values.replicas }
521-
: {
522-
desired_session_count: values.replicas,
523-
}),
524-
...getImageInfoFromInputInEditing(
525-
checkManualImageAllowed(
526-
baiClient._config.allow_manual_image_name_for_session,
527-
values.environments?.manual,
528-
),
529-
values,
481+
const mutationVariables: ServiceLauncherPageContentModifyMutation['variables'] =
482+
{
483+
endpoint_id: endpoint?.endpoint_id || '',
484+
props: {
485+
resource_slots: JSON.stringify({
486+
cpu: values.resource.cpu,
487+
mem: values.resource.mem,
488+
...(values.resource.accelerator > 0
489+
? {
490+
[values.resource.acceleratorType]:
491+
values.resource.accelerator,
492+
}
493+
: undefined),
494+
}),
495+
resource_opts: JSON.stringify({
496+
shmem: values.resource.shmem,
497+
}),
498+
// FIXME: temporarily convert cluster mode string according to server-side type
499+
cluster_mode:
500+
'single-node' === values.cluster_mode
501+
? 'SINGLE_NODE'
502+
: 'MULTI_NODE',
503+
cluster_size: values.cluster_size,
504+
...(baiClient.supports('replicas')
505+
? { replicas: values.replicas }
506+
: {
507+
desired_session_count: values.replicas,
508+
}),
509+
...getImageInfoFromInputInEditing(
510+
checkManualImageAllowed(
511+
baiClient._config.allow_manual_image_name_for_session,
512+
values.environments?.manual,
530513
),
531-
extra_mounts: _.map(values.mount_ids, (vfolder) => {
532-
return {
533-
vfolder_id: vfolder,
534-
...(values.mount_id_map[vfolder] && {
535-
mount_destination: values.mount_id_map[vfolder],
536-
}),
537-
};
538-
}),
539-
name: values.serviceName,
540-
resource_group: values.resourceGroup,
541-
model_definition_path: values.modelDefinitionPath,
542-
runtime_variant: values.runtimeVariant,
543-
},
544-
};
545-
const newEnvirons: { [key: string]: string } = {};
546-
if (values.envvars) {
547-
values.envvars.forEach(
548-
(v) => (newEnvirons[v.variable] = v.value),
549-
);
550-
}
551-
mutationVariables.props.environ = JSON.stringify(newEnvirons);
552-
commitModifyEndpoint({
553-
variables: mutationVariables,
554-
onCompleted: (res, errors) => {
555-
if (!res.modify_endpoint?.ok) {
556-
message.error(res.modify_endpoint?.msg);
557-
return;
558-
}
559-
if (errors && errors?.length > 0) {
560-
const errorMsgList = _.map(errors, (error) => error.message);
561-
for (const error of errorMsgList) {
562-
message.error(error);
563-
}
564-
} else {
565-
const updatedEndpoint = res.modify_endpoint?.endpoint;
566-
message.success(
567-
t('modelService.ServiceUpdated', {
568-
name: updatedEndpoint?.name,
514+
values,
515+
),
516+
extra_mounts: _.map(values.mount_ids, (vfolder) => {
517+
return {
518+
vfolder_id: vfolder,
519+
...(values.mount_id_map[vfolder] && {
520+
mount_destination: values.mount_id_map[vfolder],
569521
}),
570-
);
571-
webuiNavigate(`/serving/${endpoint?.endpoint_id}`);
572-
}
573-
},
574-
onError: (error) => {
575-
if (error.message) {
576-
message.error(error.message);
577-
} else {
578-
message.error(t('modelService.FailedToUpdateService'));
579-
}
522+
};
523+
}),
524+
name: values.serviceName,
525+
resource_group: values.resourceGroup,
526+
model_definition_path: values.modelDefinitionPath,
527+
runtime_variant: values.runtimeVariant,
580528
},
581-
});
582-
} else {
583-
legacyMutationToUpdateService.mutate(values, {
584-
onSuccess: () => {
529+
};
530+
const newEnvirons: { [key: string]: string } = {};
531+
if (values.envvars) {
532+
values.envvars.forEach((v) => (newEnvirons[v.variable] = v.value));
533+
}
534+
mutationVariables.props.environ = JSON.stringify(newEnvirons);
535+
commitModifyEndpoint({
536+
variables: mutationVariables,
537+
onCompleted: (res, errors) => {
538+
if (res.modify_endpoint?.ok) {
539+
const updatedEndpoint = res.modify_endpoint?.endpoint;
585540
message.success(
586541
t('modelService.ServiceUpdated', {
587-
name: endpoint.name, // FIXME: temporally get name from endpoint, not input value
542+
name: updatedEndpoint?.name,
588543
}),
589544
);
590545
webuiNavigate(`/serving/${endpoint?.endpoint_id}`);
591-
},
592-
onError: (error) => {
546+
return;
547+
}
548+
549+
if (res.modify_endpoint?.msg) {
550+
message.error(res.modify_endpoint?.msg);
551+
} else if (errors && errors?.length > 0) {
552+
const errorMsgList = _.map(errors, (error) => error.message);
553+
for (const error of errorMsgList) {
554+
message.error(error);
555+
}
556+
}
557+
},
558+
onError: (error) => {
559+
if (error.message) {
560+
message.error(error.message);
561+
} else {
593562
message.error(t('modelService.FailedToUpdateService'));
594-
},
595-
});
596-
}
563+
}
564+
},
565+
});
597566
} else {
598567
// create service
599568
mutationToCreateService.mutate(values, {

0 commit comments

Comments
 (0)