<feature>[error code]: global error code #2699
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves: ZSTAC-66343 Change-Id: I61787263626f646b6a6b6c6262666a6962787866
Walkthrough该PR在全局范围内将原有的纯文本错误构造统一切换为带“全局错误码”的错误构造：新增并使用 CloudOperationsErrorCode 代码集，扩展 Platform 错误助手方法签名以携带 globalErrorCode，并在各模块将 operr/argerr/err 等调用替换为带代码版本。另在 ErrorCode 中新增 globalErrorCode 字段。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Module as Any Module
participant Platform as Platform(err/operr/argerr…)
participant ErrorCode as ErrorCode
Caller->>Module: 触发校验/操作
Module->>Platform: operr(GLOBAL_CODE, "message", args…)
note over Module,Platform: 统一新增 globalErrorCode 入参
Platform->>ErrorCode: setGlobalErrorCode(GLOBAL_CODE)<br/>setCode(Enum or nested)<br/>setDetails(message, args)
ErrorCode-->>Module: 返回带全局码的 ErrorCode
Module-->>Caller: 抛出/返回带代码的错误
sequenceDiagram
autonumber
actor Dev
participant Core as header/ErrorCode
participant Platform as core/Platform
participant Modules as compute/*, network/*, ...
Dev->>Core: 新增 ErrorCode.globalErrorCode 字段
Dev->>Platform: 修改所有 err/operr/argerr/... 签名增加 globalErrorCode
Dev->>Modules: 扫描替换调用，传入 CloudOperationsErrorCode 常量
note over Modules: 业务逻辑不变，仅错误构造携带全局错误码
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 109
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (58)
core/src/main/java/org/zstack/core/debug/DebugManagerImpl.java (1)
103-113: 潜在 NPE 与类型不安全：为 tasks 增加 null 保护并使用泛型；统一命名风格当 getPendingTask()/getRunningTask() 返回 null 时，当前
if (!Tasks.isEmpty())会触发 NPE；且使用了 raw List 与非常规参数名。请加 null/empty 保护、采用泛型与 lowerCamelCase 命名，并对 correlation id 做空值保护。-private void replyAllMsgFromTasks(List Tasks) { - if (!Tasks.isEmpty()) { - Tasks.forEach(task -> { - //If the headers do not exist, it means that the task does not have a corresponding message - if (!((TaskInfo) task).getContext().contains(HEADERS)){ - return; - } - bus.cancel(getCorrelationIdFromTask(((TaskInfo) task).getContext()), "cancel for clean queue"); - }); - } -} +private void replyAllMsgFromTasks(List<TaskInfo> tasks) { + if (tasks == null || tasks.isEmpty()) { + return; + } + tasks.forEach(task -> { + String context = task.getContext(); + // If the headers do not exist, it means that the task does not have a corresponding message + if (context == null || !context.contains(HEADERS)) { + return; + } + String cid = getCorrelationIdFromTask(context); + if (cid != null && !cid.isEmpty()) { + bus.cancel(cid, "cancel for clean queue"); + } else { + logger.debug("skip cancel: correlation id is null/empty"); + } + }); +}plugin/kvm/src/main/java/org/zstack/kvm/xmlhook/XmlHookBase.java (1)
192-209: 并发回调写入 ArrayList 非线程安全，存在数据竞争While 的 step 回调并行执行，bus 回调线程中对
errs的写入会竞态，可能导致列表损坏或丢失错误。最小变更修复：
- List<ErrorCode> errs = new ArrayList<>(); + List<ErrorCode> errs = Collections.synchronizedList(new ArrayList<>());可选改进：若 While 提供错误收集能力（如在回调里上报并在 done(ErrorCodeList) 汇总），可移除手动
errs并使用框架收集机制，降低并发复杂度。configuration/src/main/java/org/zstack/configuration/ConfigurationManagerImpl.java (1)
1109-1117: DiskOffering 校验使用了错误的 SystemTag 常量，且错误信息对象名不一致此处应匹配 DiskOffering 的 systemTag，但代码使用了 InstanceOffering 的常量；同时报错文案提到的是 “instanceOffering”。这是功能性问题，会导致创建磁盘规格时无法正确识别重复的用户配置标签。
请按下列方式修复：
- if (InstanceOfferingSystemTags.INSTANCE_OFFERING_USER_CONFIG.isMatch(sysTag)) { + if (DiskOfferingSystemTags.DISK_OFFERING_USER_CONFIG.isMatch(sysTag)) { if (userdataTagCount > 0) { throw new OperationFailureException(argerr( - ORG_ZSTACK_CONFIGURATION_10004, "Shouldn't be more than one systemTag for one instanceOffering.")); + ORG_ZSTACK_CONFIGURATION_10004, "Shouldn't be more than one systemTag for one diskOffering.")); }core/src/main/java/org/zstack/core/workflow/WorkFlowChain.java (2)
172-172: 拼写错误：happend → happened修正英文拼写以保持专业性。
- logger.warn(String.format("Unhandled exception happend while rolling back workflow[%s] in chain[%s]", flow.getName(), getName()), t); + logger.warn(String.format("Unhandled exception happened while rolling back workflow[%s] in chain[%s]", flow.getName(), getName()), t);
344-344: 拼写错误：Noting → Nothing微调日志文案。
- logger.debug(String.format("Noting to carry on for work flow chain[uuid:%s]", chainvo.getUuid())); + logger.debug(String.format("Nothing to carry on for work flow chain[uuid:%s]", chainvo.getUuid()));console/src/main/java/org/zstack/console/ManagementServerConsoleProxyBackend.java (1)
556-566: 修正错误码消息的占位符这里的错误消息包含两个
%d占位符，但operr(...)调用未传入rst与portRst，运行时会抛出MissingFormatArgumentException，导致流程在真正返回错误码前就崩溃。请补齐参数，避免格式化异常。- if (rst == 0) { + if (rst == 0) { trigger.next(); } else { - trigger.fail(operr(ORG_ZSTACK_CONSOLE_10003, "failed to configure consoleProxyOverriddenIp[code:%d] or consoleProxyPort[code:%d]")); + trigger.fail(operr(ORG_ZSTACK_CONSOLE_10003, + "failed to configure consoleProxyOverriddenIp[code:%d] or consoleProxyPort[code:%d]", + rst, portRst)); }plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerWeightOperator.java (2)
109-115: weight 可能为 null 触发自动拆箱 NPE；请先做非空校验并抛带码异常方法参数是 Long 包装类型，未断言非空就进行比较，会在自动拆箱时报 NPE。应在范围校验前提前失败，并返回带全局错误码的 OperationFailureException。
public void setWeight(String listenerUuid, String nicUuid, Long weight) { DebugUtils.Assert(listenerUuid != null && nicUuid != null, String.format("invalid parameter listener uuid:%s nicUuid:%s", listenerUuid, nicUuid)); + if (weight == null) { + throw new OperationFailureException(argerr( + ORG_ZSTACK_NETWORK_SERVICE_LB_10027, + "balancer weight for nic %s cannot be null", nicUuid)); + } + if ( weight < LoadBalancerConstants.BALANCER_WEIGHT_MIN || weight > LoadBalancerConstants.BALANCER_WEIGHT_MAX) { - throw new OperationFailureException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10027, "invalid balancer weight for nic:%s, %d is not in the range [%d, %d]", nicUuid, weight, LoadBalancerConstants.BALANCER_WEIGHT_MIN, LoadBalancerConstants.BALANCER_WEIGHT_MAX)); + throw new OperationFailureException(argerr( + ORG_ZSTACK_NETWORK_SERVICE_LB_10027, + "invalid balancer weight for nic %s: %d is not in the range [%d, %d]", + nicUuid, weight, LoadBalancerConstants.BALANCER_WEIGHT_MIN, LoadBalancerConstants.BALANCER_WEIGHT_MAX)); }
172-175: deleteWeight 使用了错误的 Token，导致无法按 nicUuid 删除这里将 nicUuid 赋给了 BALANCER_WEIGHT_TOKEN，匹配不到任何标签，删除操作形同虚设。应当使用 BALANCER_NIC_TOKEN，并对权重使用通配符以匹配任意权重。
- LoadBalancerSystemTags.BALANCER_WEIGHT.delete(listenerUuid, - TagUtils.tagPatternToSqlPattern(LoadBalancerSystemTags.BALANCER_WEIGHT.instantiateTag( - map(e(LoadBalancerSystemTags.BALANCER_WEIGHT_TOKEN, nicUuid)) - ))); + LoadBalancerSystemTags.BALANCER_WEIGHT.delete(listenerUuid, + TagUtils.tagPatternToSqlPattern( + LoadBalancerSystemTags.BALANCER_WEIGHT.instantiateTag( + map( + e(LoadBalancerSystemTags.BALANCER_NIC_TOKEN, nicUuid), + e(LoadBalancerSystemTags.BALANCER_WEIGHT_TOKEN, "%") + ) + ) + ));core/src/main/java/org/zstack/core/salt/SaltSetupMinionJob.java (1)
156-156: 英文字符串中包含全角中文标点“：”需改为半角冒号“:”根据规范“代码里不应当有中文”，请替换为半角冒号并优化文案。
- logger.warn(String.format("FileInputStream close IOException：%s", e.getMessage())); + logger.warn(String.format("Failed to close FileInputStream: %s", e.getMessage()));As per coding guidelines
core/src/main/java/org/zstack/core/plugin/PluginManagerImpl.java (3)
238-262: 可能的空指针异常：plugin 未注册时直接访问 features()当传入的 pluginProductKey 不存在时，
pluginInstances.get(pluginProductKey)为 null，会在后续.features()调用处触发 NPE。建议先取本地变量做空值分支处理，并减少重复 Map 访问。- public boolean isFeatureSupported(String pluginProductKey, - String capability) { - if (logger.isTraceEnabled()) { - logger.trace(String.format("check plugin[%s] capability[%s]", - pluginProductKey, capability)); - logger.trace(String.format("plugin features %s", - JSONObjectUtil.toJsonString(pluginInstances - .get(pluginProductKey) - .features()))); - logger.trace(String.format("plugin feature state: %s", - JSONObjectUtil.toJsonString(pluginInstances - .get(pluginProductKey) - .features().get(capability) == Boolean.TRUE))); - } - - if (pluginInstances.get(pluginProductKey) - .features() == null) { - return true; - } - - return pluginInstances.get(pluginProductKey) - .features() - .get(capability) == Boolean.TRUE; - } + public boolean isFeatureSupported(String pluginProductKey, String capability) { + PluginDriver driver = pluginInstances.get(pluginProductKey); + if (driver == null) { + return false; + } + if (logger.isTraceEnabled()) { + logger.trace(String.format("check plugin[%s] capability[%s]", pluginProductKey, capability)); + logger.trace(String.format("plugin features %s", JSONObjectUtil.toJsonString(driver.features()))); + logger.trace(String.format("plugin feature state: %s", + JSONObjectUtil.toJsonString(driver.features() != null && driver.features().get(capability) == Boolean.TRUE))); + } + if (driver.features() == null) { + return true; + } + return driver.features().get(capability) == Boolean.TRUE; + }
279-284: 可能的空指针异常：isPluginTypeExist 对空集合未做防御
pluginRegisters.get(pluginClass)可能为 null，直接.stream()会 NPE。- public boolean isPluginTypeExist(Class<? extends PluginDriver> pluginClass, String type) { - return pluginRegisters.get(pluginClass) - .stream() - .anyMatch(plugin -> plugin.type().equals(type)); - } + public boolean isPluginTypeExist(Class<? extends PluginDriver> pluginClass, String type) { + List<PluginDriver> list = pluginRegisters.get(pluginClass); + if (list == null) { + return false; + } + return list.stream().anyMatch(plugin -> plugin.type().equals(type)); + }
287-300: 空指针与重复遍历：对列表做空值保护并只遍历一次该方法对相同列表遍历两次，且未判空。合并为一次遍历并判空更稳健。
- public <T extends PluginDriver> T getPlugin(Class<? extends PluginDriver> pluginClass, String type) { - if (pluginRegisters.get(pluginClass) - .stream() - .filter(plugin -> plugin.type().equals(type)) - .count() > 1) { - throw new CloudRuntimeException(String.format("multi plugin with same type %s", type)); - } - - return (T) pluginRegisters.get(pluginClass) - .stream() - .filter(plugin -> plugin.type().equals(type)) - .findFirst() - .orElse(null); - } + public <T extends PluginDriver> T getPlugin(Class<? extends PluginDriver> pluginClass, String type) { + List<PluginDriver> list = pluginRegisters.get(pluginClass); + if (list == null || list.isEmpty()) { + return null; + } + PluginDriver found = null; + for (PluginDriver plugin : list) { + if (plugin.type().equals(type)) { + if (found != null) { + throw new CloudRuntimeException(String.format("multi plugin with same type %s", type)); + } + found = plugin; + } + } + return (T) found; + }plugin/eip/src/main/java/org/zstack/network/service/eip/EipManagerImpl.java (2)
203-205: 逻辑错误：应当在双协议已占用时直接返回空列表，但当前未返回if 分支仅创建了新列表未返回，导致后续逻辑继续执行，和预期不符；在“Flat 模式且 IPv4/IPv6 都已占用”时会错误返回候选 EIP。
- if(isIpv4Only && isIpv6Only){ - new ArrayList<>(); - } + if (isIpv4Only && isIpv6Only) { + return new ArrayList<>(); + }
382-387: SQL 注入风险：字符串拼接用户输入（vmName/vmUuid）到原生 SQL目前使用字符串拼接构造 like 条件，存在注入风险。请改为命名参数，并对 like 特殊字符进行转义。
- if (!StringUtils.isEmpty(msg.getVmName())) { - sqlBuilder.append(" and vm.name like '%").append(msg.getVmName()).append("%\'"); - } - if (!StringUtils.isEmpty(msg.getVmUuid())) { - sqlBuilder.append(" and vm.uuid like '%").append(msg.getVmUuid()).append("%\'"); - } + if (!StringUtils.isEmpty(msg.getVmName())) { + sqlBuilder.append(" and vm.name like :vmName escape '\\\\'"); + } + if (!StringUtils.isEmpty(msg.getVmUuid())) { + sqlBuilder.append(" and vm.uuid like :vmUuid escape '\\\\'"); + }为配合上述修改，请在创建 Query 后设置参数（并转义
%、_、\）：// 追加到本方法中（示例） Map<String, Object> params = new HashMap<>(); if (!StringUtils.isEmpty(msg.getVmName())) { params.put("vmName", "%" + escapeLike(msg.getVmName()) + "%"); } if (!StringUtils.isEmpty(msg.getVmUuid())) { params.put("vmUuid", "%" + escapeLike(msg.getVmUuid()) + "%"); } Query q = dbf.getEntityManager().createNativeQuery(sqlBuilder.toString()); params.forEach(q::setParameter);并新增一个工具方法：
private static String escapeLike(String s) { return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_"); }core/src/main/java/org/zstack/core/upgrade/UpgradeChecker.java (2)
410-412: 潜在 NPE：直接对可能为 null 的版本做 equals历史数据可能存在
currentVersion或expectVersion为空的情况（文件内其他方法与注释已提示早期版本可能缺失版本元数据），这里的agent.getCurrentVersion().equals(...)存在 NPE 风险。- if (agent.getCurrentVersion().equals(agent.getExpectVersion())) { + if (java.util.Objects.equals(agent.getCurrentVersion(), agent.getExpectVersion())) { return; }建议同时在异常分支加入更友好的提示（如提示先触发一次 reconnect 以上报版本）。As per coding guidelines.
206-215: 使用 THREAD_CONTEXT_TASK_NAME 进行灰度白名单校验
- 将
if (ThreadContext.containsKey(Constants.THREAD_CONTEXT_API))修改为if (ThreadContext.containsKey(Constants.THREAD_CONTEXT_TASK_NAME))，确保基于 TASK_NAME（类名）执行灰度校验- 将返回信息中的
Api:改为API:core/src/main/java/org/zstack/core/ansible/PrepareAnsible.java (1)
36-54: 静态初始化中错误码被吞没，违背“全局错误码”目标在静态代码块内抛出的 OperationFailureException 会被下方 catch(Exception) 包装成 CloudRuntimeException，导致 ErrorCode 丢失，外层无法感知 globalErrorCode。请单独捕获并原样抛出 OperationFailureException。
- } catch (Exception e) { - throw new CloudRuntimeException(e); - } + } catch (OperationFailureException ofe) { + // preserve global error code + throw ofe; + } catch (Exception e) { + throw new CloudRuntimeException(e); + }core/src/main/java/org/zstack/core/ansible/SshFolderMd5Checker.java (1)
151-154: 日志占位参数顺序错误，信息混乱String.format 中占位符与参数顺序不匹配，当前输出会把“文件数”和“目标目录路径”等互换。请调整顺序。
-logger.debug(String.format("file number mismatch. the source folder[%s] contains %s files while the destination" + - " folder[%s] on the host[ip:%s] contains %s files. Need to deploy the agent", srcFolder, dstFolder, - srcMd5sum.size(), hostname, dstMd5sum.size())); +logger.debug(String.format("file number mismatch. the source folder[%s] contains %s files while the destination" + + " folder[%s] on the host[ip:%s] contains %s files. Need to deploy the agent", + srcFolder, srcMd5sum.size(), dstFolder, hostname, dstMd5sum.size()));plugin/directory/src/main/java/org/zstack/directory/DirectoryBase.java (3)
260-263: 潜在 NPE 与未 trim 的输入：比较与赋值前应标准化名称
msg.getName().equals(vo.getName())在msg.getName()为null时会 NPE，且未对来自 Message 的参数做trim。建议先做标准化后再比较与使用。依据规范“注意检查来自 Message 的参数是否做过 trim”。
可按如下方式调整（片段示例）：
- if (msg.getName().equals(vo.getName())) { + // normalize input + String newName = StringUtils.hasText(msg.getName()) ? msg.getName().trim() : null; + if (java.util.Objects.equals(newName, vo.getName())) { completion.success(); return; }并在后续使用处将
msg.getName()替换为newName：- List<DirectoryVO> list = directoryVOS.stream() - .filter(s -> s.getName().equals(msg.getName()) && s.getType().equals(vo.getType())) + List<DirectoryVO> list = directoryVOS.stream() + .filter(s -> java.util.Objects.equals(s.getName(), newName) && s.getType().equals(vo.getType())) .collect(Collectors.toList()); ... - vo.setName(msg.getName()); + vo.setName(newName);提示：需在 imports 显式增加：
import java.util.Objects;
344-359: 移动目录缺少“自指向”校验：可能造成直接环路当前仅校验目标父目录是否在子孙列表中，但未禁止把目录移动到自身之下（
targetParentUuid == directoryUuid）。这会形成直接环路并导致后续遍历/路径构建异常。建议在构建子孙集合前增加显式校验：
private void moveDirectory(APIMoveDirectoryMsg msg, Completion completion) { DirectoryVO vo = dbf.findByUuid(msg.getDirectoryUuid(), DirectoryVO.class); + // prevent self-parent + if (msg.getDirectoryUuid().equals(msg.getTargetParentUuid())) { + completion.fail(operr(ORG_ZSTACK_DIRECTORY_10006, + "cannot move a directory under itself: %s", msg.getDirectoryUuid())); + return; + } //avoid looping List<String> list = new ArrayList<>();
360-366: 移动到根目录时可能 NPE，且未同步更新子孙节点 groupName；目标父目录下未做重名校验
- 若
targetParentUuid == null，parentVO为null，随后parentVO.getGroupName()会 NPE。- 仅更新了当前节点的
groupName，未更新所有子孙节点路径，导致层级路径不一致。- 未校验目标父目录同级下是否已存在同名同类型目录，可能破坏同级唯一性约束。
建议最小修复如下：
- vo.setParentUuid(msg.getTargetParentUuid()); - DirectoryVO parentVO = Q.New(DirectoryVO.class).eq(DirectoryVO_.uuid, msg.getTargetParentUuid()).find(); - String newGroupName = String.format("%s/%s", parentVO.getGroupName(), vo.getName()); - vo.setGroupName(newGroupName); - dbf.update(vo); + // 目标父目录下重名校验（同级同类型同名） + List<DirectoryVO> siblings = msg.getTargetParentUuid() == null + ? Q.New(DirectoryVO.class).isNull(DirectoryVO_.parentUuid).list() + : Q.New(DirectoryVO.class).eq(DirectoryVO_.parentUuid, msg.getTargetParentUuid()).list(); + boolean dup = siblings.stream() + .anyMatch(s -> s.getType().equals(vo.getType()) && s.getName().equals(vo.getName())); + if (dup) { + completion.fail(operr(ORG_ZSTACK_DIRECTORY_10005, + "duplicate directory name under the target parent, directory with name '%s' already exists", vo.getName())); + return; + } + + String oldGroupName = vo.getGroupName(); + DirectoryVO parentVO = msg.getTargetParentUuid() == null + ? null + : Q.New(DirectoryVO.class).eq(DirectoryVO_.uuid, msg.getTargetParentUuid()).find(); + String newGroupName = (parentVO == null) + ? vo.getName() + : String.format("%s/%s", parentVO.getGroupName(), vo.getName()); + + vo.setParentUuid(msg.getTargetParentUuid()); + vo.setGroupName(newGroupName); + dbf.update(vo); + // 同步更新所有子孙目录的 groupName 前缀 + updateChildrenGroupName(oldGroupName, newGroupName, msg.getDirectoryUuid());请注意：
updateChildrenGroupName(String oldGroupName, String newGroupName, String uuid)的uuid参数当前未在方法体内使用，可后续考虑移除以简化签名（非本次必须）。identity/src/main/java/org/zstack/identity/QuotaUtil.java (1)
83-91: 错误信息少了一个空格；建议合并字符串避免再次出现拼接疏漏当前会输出 “quota exceeding.The …”。请在句号后补空格，或直接合并为单个字符串常量。
- "quota exceeding." + + "quota exceeding. " + "The resource owner(or target resource owner) account[uuid: %s name: %s] exceeds a quota[name: %s, value: %s], " +可选：方法名
CheckQuota不符合 lowerCamelCase（编码规范第2节），后续可考虑更名为checkQuota（保持对外兼容可先内部私有化或增加别名方法）。依据编码规范。identity/src/main/java/org/zstack/identity/AccountInterceptor.java (1)
51-55: APICheckPasswordMessage 密码字段需添加 noTrim 注解
APICheckPasswordMessage 中未对 password 参数标记@APIParam(noTrim = true)，框架默认会在validate()中 trim 所有字符串，可能导致密码首尾空白被移除，请在 header/src/main/java/org/zstack/header/message/APICheckPasswordMessage.java 的 password 字段或 getter 上加上@APIParam(noTrim = true)。core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java (1)
779-784: 日志中“remaining %sms”计算符号反了now - expired 会得到负值；应为 expired - now，并建议下限为 0，避免出现负数日志。
- String info = String.format("still unable to echo %s, remaining %sms to timeout. %s", url, now - expired, t.getMessage()); + long remaining = Math.max(0, expired - now); + String info = String.format("still unable to echo %s, remaining %sms to timeout. %s", url, remaining, t.getMessage());plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java (2)
638-647: 修复心跳卷创建时的潜在 NPE：当无可用池时 allocateFreePool 可能返回 null当前直接调用 allocateFreePool(size).getId()，在无可用池时会触发 NPE。应与其它路径（allocateSpace/createVolume）一致，返回带全局错误码的失败。建议如下修改：
VolumeModule heartbeatVol = apiHelper.queryVolume(iscsiHeartbeatVolumeName); if (heartbeatVol == null) { long size = SizeUnit.GIGABYTE.toByte(2); - heartbeatVol = apiHelper.createVolume(iscsiHeartbeatVolumeName, allocateFreePool(size).getId(), size); + FailureDomainModule pool = allocateFreePool(size); + if (pool == null) { + comp.fail(operr(ORG_ZSTACK_EXPON_10009, + "no available pool with enough space[%d] and healthy status for heartbeat volume", size)); + return; + } + heartbeatVol = apiHelper.createVolume(iscsiHeartbeatVolumeName, pool.getId(), size); }As per coding guidelines
1149-1158: 统一“协议不支持”错误处理并修正文案（unsupport → unsupported）export(...) 在不支持协议时仍抛 RuntimeException 且文案有拼写问题，应与其它路径一致使用全局错误码并通过 comp.fail 返回：
- } else { - throw new RuntimeException("unsupport target " + protocol.name()); - } + } else { + comp.fail(operr(ORG_ZSTACK_EXPON_10011, "unsupported protocol %s", protocol.name())); + return; + }As per coding guidelines
plugin/ceph/src/main/java/org/zstack/storage/ceph/MonUri.java (1)
96-101: 捕获并校验 monPort 解析异常和范围
在 plugin/ceph/src/main/java/org/zstack/storage/ceph/MonUri.java 中，将monPort = v == null ? monPort : Integer.parseInt(v);替换为：
if (v != null) { try { monPort = Integer.parseInt(v); } catch (NumberFormatException e) { throw new OperationFailureException( operr(ORG_ZSTACK_STORAGE_CEPH_10006, "invalid monUrl[%s], monPort 必须为整数。", urlForLog) ); } if (monPort < 1 || monPort > 65535) { throw new OperationFailureException( operr(ORG_ZSTACK_STORAGE_CEPH_10006, "invalid monUrl[%s], monPort 必须在 [1, 65535] 范围内。", urlForLog) ); } }复用已存在的
ORG_ZSTACK_STORAGE_CEPH_10006并更新错误文案即可。identity/src/main/java/org/zstack/identity/AccountManagerImpl.java (1)
408-412: 拼接字符串缺少空格，导致 “user'spermissions” 连写第二段拼接缺少前导空格，建议合并为一个片段或补空格。
- ORG_ZSTACK_IDENTITY_10025, "the user specified by the userUuid[%s] does not belong to the current account, and the" + - " current account is not an admin account, so it has no permission to check the user's" + - "permissions", msg.getUserUuid() + ORG_ZSTACK_IDENTITY_10025, "the user specified by the userUuid[%s] does not belong to the current account, and the" + + " current account is not an admin account, so it has no permission to check the user's permissions", + msg.getUserUuid()plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephSnapshotProtector.java (2)
27-31: 避免二次回调：completion.success()后缺少return当前在第三方平台标签命中时仅调用了
completion.success()，但未返回，后续分支仍可能继续执行并再次回调，存在“双重完成/失败”风险。建议最小修复如下：
if (irregularThirdPartySnapshotInstallPath) { completion.success(); + return; }
24-26: 添加primaryStorageInstallPath空值判断
primaryStorageInstallPath在VolumeSnapshotInventory上标注了@nullable，直接调用snapshot.getPrimaryStorageInstallPath().contains(...)会有 NPE 风险。- 建议在
protect方法中、调用contains之前增加：String installPath = snapshot.getPrimaryStorageInstallPath(); if (installPath == null) { completion.success(); return; }snapshot.getVolumeUuid()按设计不可为null，无需额外判空。network/src/main/java/org/zstack/network/service/DhcpExtension.java (1)
334-337: 统一 DhcpExtension 中异常风格
- 在
network/src/main/java/org/zstack/network/service/DhcpExtension.java的workoutDhcp(...)(约第336行) 和vmDefaultL3NetworkChanged(...)(约第379行) 中，当前使用throw new CloudRuntimeException(...)抛出未经编码的异常，应改为带全局错误码的失败路径，例如：throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_100XX, "unable to find NetworkServiceDhcpBackend[provider type: %s]", ptype));- 保留或替换原
CloudRuntimeException时，务必在异常信息中包含operr(...).toString()，并评估调用方是否依赖原异常类型与文案。plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageFactory.java (1)
1025-1027: 获取 watchers 的失败信息不够自然建议使用更自然的英文并统一大小写。
- completion.fail(operr(ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10042, "get rootVolume[%s] rbd image watchers fail, %s", - rootVolume.getInstallPath(), reply.getError().getDetails())); + completion.fail(operr(ORG_ZSTACK_STORAGE_CEPH_PRIMARY_10042, + "failed to get RBD image watchers of root volume [%s]: %s", + rootVolume.getInstallPath(), reply.getError().getDetails()));As per coding guidelines.
network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java (2)
155-166: 可能的查询逻辑漏洞：缺少 l2 与 ref 的关联条件，存在笛卡尔积风险VLAN 分支查询未包含
l2.uuid = ref.l2NetworkUuid的关联条件（NoVLAN 分支有）。可能误报冲突。- List<L2NetworkVO> l2s = SQL.New("select l2" + - " from L2NetworkVO l2, L2NetworkClusterRefVO ref" + - " where l2.virtualNetworkId = :virtualNetworkId" + - " and l2.physicalInterface = :physicalInterface" + - " and ref.clusterUuid in (:clusterUuids)" + - " and l2.type = 'L2VlanNetwork'") + List<L2NetworkVO> l2s = SQL.New("select l2" + + " from L2NetworkVO l2, L2NetworkClusterRefVO ref" + + " where l2.uuid = ref.l2NetworkUuid" + + " and l2.virtualNetworkId = :virtualNetworkId" + + " and l2.physicalInterface = :physicalInterface" + + " and ref.clusterUuid in (:clusterUuids)" + + " and l2.type = 'L2VlanNetwork'") .param("virtualNetworkId", msg.getVlan()) .param("physicalInterface", l2.getPhysicalInterface()) .param("clusterUuids", attachedClusters).list();
176-186: NoVLAN 分支 JPQL 语句别名不一致，建议显式使用 l2.type这里写成
and type = 'L2NoVlanNetwork'，在多实体查询中应显式指明别名，避免解析/歧义问题；与 VLAN 分支保持一致。- " and type = 'L2NoVlanNetwork'") + " and l2.type = 'L2NoVlanNetwork'")plugin/ldap/src/main/java/org/zstack/ldap/LdapUtil.java (1)
811-815: 【提示语可读性】错误码 ORG_ZSTACK_LDAP_10019 使用正确，但文案里 “You'd better … first.getDn” 用语不够正式且有拼写连接问题。建议调整：- "You'd better check the LDAP/AD server[url:%s, baseDN:%s, encryption:%s, username:%s, password:******] configuration and test connection first.getDn error filter:%s", + "Please verify the LDAP/AD server configuration [url:%s, baseDN:%s, encryption:%s, username:%s, password:******] and test the connection first. getDn error, filter: %s",plugin/acl/src/main/java/org/zstack/acl/AccessControlListApiInterceptor.java (1)
162-189: 修复 NPE 风险并对 Message 参数进行统一 trim（拦截器规范）
- 当 msg.getDomain() 非空而 msg.getUrl() 为 null 时，分支 else 内部直接调用 msg.getUrl().length() 会触发 NPE（Lines 177/184）。
- 按拦截器规范，应对来自 Message 的字符串做 trim，避免用户输入中包含的空格/换行导致校验误判。
建议按下述补丁在进入分支前归一化 domain/url，并全程使用局部变量，既消除 NPE，又满足 trim 要求（同时一并修复行 191 误传参问题）：
@@ - if (StringUtils.isBlank(msg.getDomain()) && (StringUtils.isBlank(msg.getUrl()) || msg.getUrl().length() == 1)) { - throw new ApiMessageInterceptionException(operr(ORG_ZSTACK_ACL_10010, "domain and url can not both empty")); - } + // normalize inputs to avoid NPEs and whitespace issues + String domain = StringUtils.trimToEmpty(msg.getDomain()); + String url = StringUtils.trimToEmpty(msg.getUrl()); + msg.setDomain(domain); + msg.setUrl(url); + if (StringUtils.isBlank(domain) && (StringUtils.isBlank(url) || url.length() == 1)) { + throw new ApiMessageInterceptionException(operr(ORG_ZSTACK_ACL_10010, "domain and url can not both empty")); + } @@ - if (StringUtils.isNotBlank(msg.getDomain())) { - if (!AccessControlListUtils.isValidateDomain(msg.getDomain())) { + if (StringUtils.isNotBlank(domain)) { + if (!AccessControlListUtils.isValidateDomain(domain)) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_ACL_10011, "domain[%s] is not validate domain", msg.getDomain())); } - if (msg.getDomain().contains("*")) { + if (domain.contains("*")) { msg.setCriterion("WildcardMatch"); } else { msg.setCriterion("AccurateMatch"); } - if (StringUtils.isNotBlank(msg.getUrl()) && msg.getUrl().length() > 1) { - if (!AccessControlListUtils.isValidateUrl(msg.getUrl())) { + if (StringUtils.isNotBlank(url) && url.length() > 1) { + if (!AccessControlListUtils.isValidateUrl(url)) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_ACL_10012, "url[%s] is not validate url", msg.getUrl())); } msg.setMatchMethod("DomainAndUrl"); } else { - if (msg.getUrl().length() == 1 && !msg.getUrl().equals("/")) { + if (url.length() == 1 && !"/".equals(url)) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_ACL_10013, "url[%s] is not validate url", msg.getUrl())); } msg.setUrl("/"); msg.setMatchMethod("Domain"); } } else { - if (!AccessControlListUtils.isValidateUrl(msg.getUrl())) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_ACL_10014, "url[%s] is not validate url", msg.getDomain())); + if (!AccessControlListUtils.isValidateUrl(url)) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_ACL_10014, "url[%s] is not validate url", url)); } msg.setMatchMethod("Url"); }core/src/main/java/org/zstack/core/errorcode/ElaborationManagerImpl.java (2)
101-114: 目录扫描失败与空目录提示需改进英文并补充上下文
- 失败信息建议包含被扫描路径；语法从 “is not existed” 改为 “does not exist”。
- trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10002, "Unable to scan folder: %s", e.getMessage())); + trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10002, "failed to scan folder '%s': %s", folder.getAbsolutePath(), e.getMessage())); - trigger.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10003, "%s is not existed or is empty folder", filename)); + trigger.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10003, "'%s' does not exist or is an empty folder", filename));As per coding guidelines
144-151: 读取模板文件建议显式字符集，错误文案与调用方式统一
- 使用指定字符集避免平台默认编码差异。
- 统一使用静态导入的 operr，并简化 String.format。
- 改进英文措辞。
- String content = FileUtils.readFileToString(templateFile); + String content = FileUtils.readFileToString(templateFile, java.nio.charset.StandardCharsets.UTF_8); @@ - trigger.fail(Platform.operr(ORG_ZSTACK_CORE_ERRORCODE_10004, String.format("read error elaboration template files [%s] failed, due to: %s", templateFile, e.getMessage()))); + trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10004, + "failed to read elaboration template file '%s': %s", + templateFile.getAbsolutePath(), e.getMessage()));提示：上述改动需要确保导入
java.nio.charset.StandardCharsets（或使用全限定名如上）。
As per coding guidelinescompute/src/main/java/org/zstack/compute/cluster/ClusterApiInterceptor.java (1)
53-60: 存在潜在 NPE 与冗长条件判断：对msg.getHypervisorType()未做空值保护，msg.getHypervisorType().equals("baremetal")会在缺失参数时触发 NPE；同时当前双向判断可简化为“两个标志不一致”。建议如下修复，避免 NPE 并降低复杂度。- if ((msg.getType() != null && msg.getType().equals("baremetal") && !msg.getHypervisorType().equals("baremetal")) || - (msg.getHypervisorType().equals("baremetal") && msg.getType() != null && !msg.getType().equals("baremetal"))) { + boolean typeBare = "baremetal".equalsIgnoreCase(msg.getType()); + boolean hvBare = "baremetal".equalsIgnoreCase(msg.getHypervisorType()); + if (typeBare ^ hvBare) { throw new ApiMessageInterceptionException(Platform.argerr( ORG_ZSTACK_COMPUTE_CLUSTER_10003, "if cluster type is baremetal, then hypervisorType must be baremetal too, or vice versa" )); }符合“避免 NPE”的异常处理与条件简化规范。As per coding guidelines
core/src/main/java/org/zstack/core/job/JobQueueFacadeImpl2.java (1)
294-299: 潜在空指针：findJob 返回 null 后仍继续 while 循环当反序列化失败并标记当前任务为 Error 后，
findJob(qvo)可能返回 null；随后下一轮循环对jobe.getContext()会触发 NPE。建议在 catch 分支内对 null 做提前返回并释放队列。- jobFail(jobe, ierr); - logger.warn(ierr.getDetails(), e1); - jobe = findJob(qvo); + jobFail(jobe, ierr); + logger.warn(ierr.getDetails(), e1); + jobe = findJob(qvo); + if (jobe == null) { + // no more pending job after marking current one Error, release queue + dbf.remove(qvo); + logger.debug(String.format("[JobQueue released, no pending task after error, delete the queue] last owner: %s, queue name: %s, queue id: %s", + qvo.getOwner(), qvo.getName(), qvo.getId())); + return null; + }plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (4)
4046-4050: 错误对象 ID 填写有误：应为 hostUuid 而非 vmUuid报错文案写“host[uuid:%s]”，实际传入的是 vm.getUuid()，将导致排障误导。
- ErrorCode err = operr(ORG_ZSTACK_KVM_10081, - "host[uuid:%s] capacity is not enough to offer cpu[%s], memory[%s bytes]", - vm.getUuid(), cpuChangeTo - oldCpuNum, alignedMemory.get() - oldMemorySize); + ErrorCode err = operr(ORG_ZSTACK_KVM_10081, + "host[uuid:%s] capacity is not enough to offer cpu[%s], memory[%s bytes]", + self.getUuid(), cpuChangeTo - oldCpuNum, alignedMemory.get() - oldMemorySize);
6275-6286: 单位不一致：消息称“seconds”实际传入毫秒ctimeout 为毫秒，应回传 timeoutInSec 或改文案为“milliseconds”。建议用秒保持一致。
- reply.setError(operr(ORG_ZSTACK_KVM_10126, - "host[%s] not shutdown in %d seconds", msg.getHostUuid(), ctimeout)); + reply.setError(operr(ORG_ZSTACK_KVM_10126, + "host[%s] not shutdown in %d seconds", msg.getHostUuid(), timeoutInSec));
2991-2994: 文案双重否定（doesn't not）需更正用户可见文本应避免语法错误。
- "kvm host[uuid:%s, name:%s, ip:%s] doesn't not support live snapshot. please stop vm[uuid:%s] and try again" + "kvm host[uuid:%s, name:%s, ip:%s] does not support live snapshot. please stop vm[uuid:%s] and try again"
4841-4844: 日志占位符未替换当前日志含“%s”未提供参数，应该格式化输出状态值。
- logger.debug("host status is %s, ignore version or host uuid changed issue"); + logger.debug(String.format("host status is %s, ignore version or host uuid changed issue", self.getStatus()));compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java (1)
324-405: 功能性缺陷：newSystags 在循环外定义，导致重复添加 systemTagsnewSystags 在进入循环前创建并在每次迭代末尾整体 addAll，会把之前迭代生成的标签重复追加，多次迭代将出现重复 system tag。
请将 newSystags 的作用域下移到每次迭代内，或在追加后清空：
- List<String> newSystags = new ArrayList<>(); - for (Map.Entry<String, NicIpAddressInfo> e : staticIps.entrySet()) { + for (Map.Entry<String, NicIpAddressInfo> e : staticIps.entrySet()) { + List<String> newSystags = new ArrayList<>(); String l3Uuid = e.getKey(); NicIpAddressInfo nicIp = e.getValue(); ... - if (!newSystags.isEmpty()) { - msg.getSystemTags().addAll(newSystags); - } + if (!newSystags.isEmpty()) { + msg.getSystemTags().addAll(newSystags); + } }或保留原位置但在 addAll 之后 newSystags.clear()。基于过往 Learning：系统标签流程应避免重复标签引起的二次验证异常。
header/src/main/java/org/zstack/header/errorcode/ErrorCode.java (1)
76-85: 拷贝构造未复制 globalErrorCode，导致新字段丢失使用拷贝构造生成的新 ErrorCode 将丢失 globalErrorCode，破坏全局错误码传播链路。
public ErrorCode(ErrorCode other) { this.code = other.code; this.description = other.description; this.details = other.details; this.elaboration = other.elaboration; this.messages = other.messages; this.cause = other.cause; this.location = other.location; + this.globalErrorCode = other.globalErrorCode; }compute/src/main/java/org/zstack/compute/host/HostBase.java (1)
288-296: 潜在NPE：ipmi 可能为 null
ipmi = dbf.findByUuid(..., HostIpmiVO.class)可能返回空，随后直接ipmi.getIpmiAddress()将抛NullPointerException。建议空指针防护并合并判断：
- HostIpmiVO ipmi = dbf.findByUuid(msg.getHostUuid(), HostIpmiVO.class); - if (null == ipmi.getIpmiAddress()) { + HostIpmiVO ipmi = dbf.findByUuid(msg.getHostUuid(), HostIpmiVO.class); + if (ipmi == null || ipmi.getIpmiAddress() == null) { event.setSuccess(false); event.setError(operr(ORG_ZSTACK_COMPUTE_HOST_10028, "host[%s] does not have ipmi device or ipmi does not have address." + "After config ipmi address, please reconnect host to refresh " + "host ipmi information", msg.getHostUuid())); bus.publish(event); return; }此外，建议将提示语精炼为“After configuring the IPMI address, please reconnect the host to refresh IPMI information.”
core/src/main/java/org/zstack/core/externalservice/ExternalServiceManagerImpl.java (1)
31-38: 修复 registerService 方法中的并发竞态
registerService 使用 containsKey + put 在 ConcurrentHashMap 上并非原子操作，两个线程可同时通过检查并互相覆盖或漏抛异常。
将逻辑改为使用 putIfAbsent 保证原子性：- if (services.containsKey(service.getName())) { - throw new OperationFailureException(operr(ORG_ZSTACK_CORE_EXTERNALSERVICE_10000, - "service[%s] has been registered", service.getName())); - } - - services.put(service.getName(), service); - return service; + ExternalService prev = services.putIfAbsent(service.getName(), service); + if (prev != null) { + throw new OperationFailureException(operr(ORG_ZSTACK_CORE_EXTERNALSERVICE_10000, + "service[%s] has been registered", service.getName())); + } + return service;另外，可选地将 getService(String, Supplier) 改为 computeIfAbsent 以避免竞态：
return services.computeIfAbsent(name, k -> supplier.get());compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (1)
5692-5700: 错误字段赋值：应当 setProtocol 而不是重复 setIsoInstallPath交换 ISO 的安装路径与协议时，最后一行把
protocol传入了setIsoInstallPath，导致协议字段丢失。建议修复如下：
- sourceCdRomVO.setIsoUuid(targetCdRomIsoUuid); - sourceCdRomVO.setIsoInstallPath(path); - sourceCdRomVO.setIsoInstallPath(protocol); + sourceCdRomVO.setIsoUuid(targetCdRomIsoUuid); + sourceCdRomVO.setIsoInstallPath(path); + sourceCdRomVO.setProtocol(protocol);compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java (3)
392-401: 【统一验证异常类型：请改为 ApiMessageInterceptionException】
本段为 API 入参/状态校验，当前抛出 OperationFailureException，不符合 VM 拦截器的既有约定（校验期应抛 ApiMessageInterceptionException）。建议替换，保持异常处理一致性与调用方预期。Based on learnings
- throw new OperationFailureException(argerr(ORG_ZSTACK_COMPUTE_VM_10055, "the image[name:%s, uuid:%s] is an ISO, rootDiskSize must be set", + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_VM_10055, "the image[name:%s, uuid:%s] is an ISO, rootDiskSize must be set", image.getName(), image.getUuid()));
469-474: 【统一验证异常类型：请改为 ApiMessageInterceptionException】
变更为带码后仍使用 OperationFailureException 抛校验失败，建议改为 ApiMessageInterceptionException。Based on learnings
- throw new OperationFailureException(operr(ORG_ZSTACK_COMPUTE_VM_10063, "The state of vm[uuid:%s] is %s. Only these state[%s] is allowed to update cpu or memory.", + throw new ApiMessageInterceptionException(operr(ORG_ZSTACK_COMPUTE_VM_10063, "The state of vm[uuid:%s] is %s. Only these state[%s] is allowed to update cpu or memory.", vo.getUuid(), vo.getState(), StringUtils.join(list(VmInstanceState.Running, VmInstanceState.Stopped), ",")));
1026-1032: 【JSON 解析错误的异常类型】
JSON 反序列化属于参数格式校验，建议从 OperationFailureException 切换为 ApiMessageInterceptionException。Based on learnings
- throw new OperationFailureException(operr(ORG_ZSTACK_COMPUTE_VM_10112, "invalid json format, causes: %s", e.getMessage())); + throw new ApiMessageInterceptionException(operr(ORG_ZSTACK_COMPUTE_VM_10112, "invalid json format, causes: %s", e.getMessage()));longjob/src/main/java/org/zstack/longjob/LongJobManagerImpl.java (1)
364-369: 将“功能不支持”和“状态非法”拆分处理，避免错误类型与语义混淆。把两种不同原因混在一个 NOT_SUPPORTED 中会误导调用方与统计。建议分别返回 NOT_SUPPORTED 与 INVALID_ARGUMENT（或项目内对应的“状态非法”错误）。
建议改造：
- if (!longJobFactory.supportClean(vo.getJobName()) || vo.getState() != LongJobState.Canceling) { - evt.setError(err(ORG_ZSTACK_LONGJOB_10001, LongJobErrors.NOT_SUPPORTED, "not supported or state is not Canceling")); - bus.publish(evt); - chain.next(); - return; - } + if (!longJobFactory.supportClean(vo.getJobName())) { + evt.setError(err( + ORG_ZSTACK_LONGJOB_10001, + LongJobErrors.NOT_SUPPORTED, + "clean is not supported for longjob[jobName:%s]", vo.getJobName() + )); + bus.publish(evt); + chain.next(); + return; + } + if (vo.getState() != LongJobState.Canceling) { + evt.setError(argerr( + ORG_ZSTACK_LONGJOB_10001, + "invalid state[%s], expected Canceling for clean", vo.getState() + )); + bus.publish(evt); + chain.next(); + return; + }并补充导入（文件头部）：
+import static org.zstack.core.Platform.argerr;As per coding guidelines
compute/src/main/java/org/zstack/compute/host/HostIpmiPowerExecutor.java (2)
39-44: 修复潜在 NPE：host.getIpmi() 可能为 null；以及 refresh 路径空指针风险
- powerOff/powerOn/powerReset 在读取 host.getIpmi().getIpmiAddress()/getIpmiPort() 前未判空，若未配置 IPMI 将触发 NPE。
- refreshHostPowerStatus(HostIpmiVO) 对入参 ipmi 未判空，同样存在 NPE 风险。
建议在三个对外方法开头统一做“未配置 IPMI”快速失败，并在 refresh 方法内做空判。示例补丁：
@@ public void powerOff(HostVO host, Boolean force, Completion completion, boolean returnEarly) { + if (isIpmiUnConfigured(host.getIpmi())) { + completion.fail(operr(ORG_ZSTACK_COMPUTE_HOST_10007, "IPMI information is incomplete for host[%s].", host.getUuid())); + return; + } if (HostPowerStatus.POWER_OFF.equals(refreshHostPowerStatus(host).getIpmiPowerStatus())) { @@ public void powerOn(HostVO host, Completion completion) { + if (isIpmiUnConfigured(host.getIpmi())) { + completion.fail(operr(ORG_ZSTACK_COMPUTE_HOST_10007, "IPMI information is incomplete for host[%s].", host.getUuid())); + return; + } if (HostPowerStatus.POWER_ON.equals(refreshHostPowerStatus(host).getIpmiPowerStatus())) { @@ public void powerReset(HostVO host, Completion completion, boolean returnEarly) { + if (isIpmiUnConfigured(host.getIpmi())) { + completion.fail(operr(ORG_ZSTACK_COMPUTE_HOST_10007, "IPMI information is incomplete for host[%s].", host.getUuid())); + return; + } if (HostPowerStatus.POWER_OFF.equals(refreshHostPowerStatus(host).getIpmiPowerStatus())) { @@ public HostIpmiVO refreshHostPowerStatus(HostIpmiVO ipmi) { + if (ipmi == null) { + return null; + } HostPowerStatus status = getPowerStatus(ipmi);Also applies to: 103-108, 137-142, 88-94
70-74: 修复 Boolean 拆箱 NPE：直接使用 if (force) 存在空指针风险force 为包装类型，可能为 null。请改为空安全判断：
- if (force) { + if (Boolean.TRUE.equals(force)) { ret = caller.powerOff(); } else {plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatEipBackend.java (1)
638-642: 潜在 NPE：detach 场景下 struct.getGuestIp() 可能为 null在上方分支“for detachEip”明确当 struct.getGuestIp() 为 null，仅设置了 to.nicIp 与 to.ipVersion；此处仍访问 struct.getGuestIp().getNetmask() 将触发 NPE。此为阻断级问题。
建议改为基于已确定的 to.ipVersion 计算，并在 guestIp 为空时从 nic 的 UsedIpVO 中按 IP 匹配回退获取 netmask（若仍不可得，可记录警告或返回更明确的失败）：
- if (struct.getGuestIp().getIpVersion() == IPv6Constants.IPv4) { - to.nicPrefixLen = NetworkUtils.getPrefixLengthFromNetmask(struct.getGuestIp().getNetmask()); - } else { - to.nicPrefixLen = IPv6NetworkUtils.getPrefixLengthFromNetmask(struct.getGuestIp().getNetmask()); - } + String nicNetmask = null; + if (struct.getGuestIp() != null) { + nicNetmask = struct.getGuestIp().getNetmask(); + } else { + // detach 场景：从当前 NIC 的 UsedIp 列表中找到与 to.nicIp 匹配的记录以获取掩码 + if (struct.getNic() != null && struct.getNic().getUsedIps() != null) { + for (UsedIpVO ip : struct.getNic().getUsedIps()) { + if (to.nicIp != null && to.nicIp.equals(ip.getIp())) { + nicNetmask = ip.getNetmask(); + break; + } + } + } + } + if (nicNetmask != null) { + to.nicPrefixLen = (to.ipVersion == IPv6Constants.IPv4) + ? NetworkUtils.getPrefixLengthFromNetmask(nicNetmask) + : IPv6NetworkUtils.getPrefixLengthFromNetmask(nicNetmask); + } else { + // 无法推导掩码：视后端要求选择 fail/告警，这里建议直接 fail 更可控 + throw new CloudRuntimeException(String.format( + "cannot determine netmask for NIC[ip:%s, vmNicUuid:%s] to compute prefix length when detaching EIP", + to.nicIp, to.nicUuid)); + }如后端允许缺省前缀长度，也可改为记录 warn 并跳过设置。
plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatDhcpBackend.java (2)
2754-2757: IPv6 范围判定使用了 IPv4 的工具方法这里应调用
IPv6NetworkUtils.isIpv6InRange，否则对于 IPv6 会误判。请按下列补丁修复：
- if (NetworkUtils.isInRange(dhcp6Ip, ipr.getStartIp(), ipr.getEndIp())) { + if (IPv6NetworkUtils.isIpv6InRange(dhcp6Ip, ipr.getStartIp(), ipr.getEndIp())) {
2752-2752: 错误信息措辞微调“allocated dhcp server ip failed”表达生硬，建议改为“failed to allocate ...”并区分 IPv4/IPv6。
建议：
- if (dhcpIp == null) { - completion.fail(argerr(ORG_ZSTACK_NETWORK_SERVICE_FLAT_10052, "allocated dhcp server ip failed")); + if (dhcpIp == null) { + completion.fail(argerr(ORG_ZSTACK_NETWORK_SERVICE_FLAT_10052, "failed to allocate DHCP server IP")); } ... - if (dhcp6Ip == null) { - completion.fail(argerr(ORG_ZSTACK_NETWORK_SERVICE_FLAT_10053, "allocated dhcp server ip failed")); + if (dhcp6Ip == null) { + completion.fail(argerr(ORG_ZSTACK_NETWORK_SERVICE_FLAT_10053, "failed to allocate DHCPv6 server IP")); }Also applies to: 2765-2765
Comment on lines
36
to
38
| if (candidates.isEmpty()) { | ||
| fail(Platform.operr("after filtering, HostAllocatorFilterExtensionPoint[%s] returns zero candidate host, it means: %s", filter.getClass().getSimpleName(), filter.filterErrorReason())); | ||
| fail(Platform.operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10008, "after filtering, HostAllocatorFilterExtensionPoint[%s] returns zero candidate host, it means: %s", filter.getClass().getSimpleName(), filter.filterErrorReason())); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
fail(...) 之后缺少 return，流程可能继续执行到 next(candidates)。
上方 catch 分支在 fail(...) 后显式 return；此分支未 return，可能导致在置失败后仍继续循环/执行 next(...)，引发双重回调或状态不一致。应保持一致性并立即返回。
if (candidates.isEmpty()) {
- fail(Platform.operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10008, "after filtering, HostAllocatorFilterExtensionPoint[%s] returns zero candidate host, it means: %s", filter.getClass().getSimpleName(), filter.filterErrorReason()));
+ fail(Platform.operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10008,
+ "after filtering, HostAllocatorFilterExtensionPoint[%s] returns zero candidate host, it means: %s",
+ filter.getClass().getSimpleName(), filter.filterErrorReason()));
+ return;
}另外，请确认错误码与语义匹配且类型为 String（Platform.operr 首参为 String）。若 CloudOperationsErrorCode 为枚举/对象，则需要使用对应的字符串值。
🏁 Script executed:
#!/bin/bash
# 1) 定位错误码定义，核对其类型是否为 String
rg -nP 'class|enum\s+CloudOperationsErrorCode|ORG_ZSTACK_COMPUTE_ALLOCATOR_10008' -C3
# 2) 查看该错误码在仓库的使用语境，确认语义一致（“过滤后无候选宿主”）
rg -n 'ORG_ZSTACK_COMPUTE_ALLOCATOR_10008' -C2
# 3) 检查 FilterFlow 中所有 fail(...) 调用是否均在本方法内终止控制流
rg -n 'fail\(' compute/src/main/java/org/zstack/compute/allocator/FilterFlow.java -C2Length of output: 269432
请继续执行以下脚本，检查 FilterFlow.java 中所有 fail(...) 调用及其后续控制流是否包含 return：
🏁 Script executed:
#!/bin/bash
rg -n 'fail\(' -C2 compute/src/main/java/org/zstack/compute/allocator/FilterFlow.javaLength of output: 540
在 compute/src/main/java/org/zstack/compute/allocator/FilterFlow.java 的 if (candidates.isEmpty()) 分支，fail 调用后添加 return，保持与 catch 分支一致，防止继续执行后续逻辑。
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/allocator/FilterFlow.java around
lines 36 to 38, the if (candidates.isEmpty()) branch calls fail(...) but does
not return, allowing execution to continue; add a return immediately after the
fail(...) call so the method exits (matching the behavior in the catch branch)
and prevents subsequent logic from running when there are no candidates.
Comment on lines
55
to
60
| private void validate(APIGetCandidateBackupStorageForCreatingImageMsg msg) { | ||
| if (msg.getVolumeSnapshotUuid() == null && msg.getVolumeUuid() == null) { | ||
| throw new ApiMessageInterceptionException(argerr( | ||
| "either volumeUuid or volumeSnapshotUuid must be set" | ||
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, "either volumeUuid or volumeSnapshotUuid must be set" | ||
| )); | ||
| } |
There was a problem hiding this comment.
拦截器未对字符串参数做 trim/blank 校验，空白UUID可绕过判定
当前仅判 null，" " 这类空白字符串会误判为已设置。按拦截器规范应先 trim 再判空，并回写规范化后的值以便后续流程使用。
应用示例 diff：
- if (msg.getVolumeSnapshotUuid() == null && msg.getVolumeUuid() == null) {
- throw new ApiMessageInterceptionException(argerr(
- ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, "either volumeUuid or volumeSnapshotUuid must be set"
- ));
- }
+ final String volUuid = msg.getVolumeUuid() == null ? null : msg.getVolumeUuid().trim();
+ final String snapUuid = msg.getVolumeSnapshotUuid() == null ? null : msg.getVolumeSnapshotUuid().trim();
+ msg.setVolumeUuid(volUuid);
+ msg.setVolumeSnapshotUuid(snapUuid);
+ if (StringUtils.isBlank(volUuid) && StringUtils.isBlank(snapUuid)) {
+ throw new ApiMessageInterceptionException(argerr(
+ ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, "either volumeUuid or volumeSnapshotUuid must be set"
+ ));
+ }As per coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void validate(APIGetCandidateBackupStorageForCreatingImageMsg msg) { | |
| if (msg.getVolumeSnapshotUuid() == null && msg.getVolumeUuid() == null) { | |
| throw new ApiMessageInterceptionException(argerr( | |
| "either volumeUuid or volumeSnapshotUuid must be set" | |
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, "either volumeUuid or volumeSnapshotUuid must be set" | |
| )); | |
| } | |
| private void validate(APIGetCandidateBackupStorageForCreatingImageMsg msg) { | |
| - if (msg.getVolumeSnapshotUuid() == null && msg.getVolumeUuid() == null) { | |
| - throw new ApiMessageInterceptionException(argerr( | |
| - ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, "either volumeUuid or volumeSnapshotUuid must be set" | |
| - )); | |
| // normalize and guard against blank UUIDs | |
| final String volUuid = msg.getVolumeUuid() == null ? null : msg.getVolumeUuid().trim(); | |
| final String snapUuid = msg.getVolumeSnapshotUuid() == null ? null : msg.getVolumeSnapshotUuid().trim(); | |
| msg.setVolumeUuid(volUuid); | |
| msg.setVolumeSnapshotUuid(snapUuid); | |
| if (StringUtils.isBlank(volUuid) && StringUtils.isBlank(snapUuid)) { | |
| throw new ApiMessageInterceptionException(argerr( | |
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10033, | |
| "either volumeUuid or volumeSnapshotUuid must be set" | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
compute/src/main/java/org/zstack/compute/allocator/HostAllocatorApiInterceptor.java
around lines 55 to 60, the interceptor only checks for null but not blank UUIDs,
so strings like " " bypass validation; trim both msg.getVolumeSnapshotUuid() and
msg.getVolumeUuid(), then check for empty after trimming, and write the trimmed
values back to the msg before validation; if both are blank/empty after
trimming, throw the existing ApiMessageInterceptionException with the same
argerr message.
Comment on lines
75
to
77
| if (!pass && !msg.isAll()) { | ||
| throw new ApiMessageInterceptionException(argerr("zoneUuids, clusterUuids, hostUuids must at least have one be none-empty list, or all is set to true")); | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10034, "zoneUuids, clusterUuids, hostUuids must at least have one be none-empty list, or all is set to true")); | ||
| } |
There was a problem hiding this comment.
错误文案英语拼写与语法问题（none-empty → non-empty）并优化可读性
当前文案存在拼写错误且语义不够清晰，建议调整为更自然的表达，避免用户困惑。
应用示例 diff：
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10034, "zoneUuids, clusterUuids, hostUuids must at least have one be none-empty list, or all is set to true"));
+ throw new ApiMessageInterceptionException(argerr(
+ ORG_ZSTACK_COMPUTE_ALLOCATOR_10034,
+ "At least one of zoneUuids, clusterUuids, or hostUuids must be a non-empty list, or 'all' must be set to true"
+ ));As per coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!pass && !msg.isAll()) { | |
| throw new ApiMessageInterceptionException(argerr("zoneUuids, clusterUuids, hostUuids must at least have one be none-empty list, or all is set to true")); | |
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10034, "zoneUuids, clusterUuids, hostUuids must at least have one be none-empty list, or all is set to true")); | |
| } | |
| if (!pass && !msg.isAll()) { | |
| throw new ApiMessageInterceptionException(argerr( | |
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10034, | |
| "At least one of zoneUuids, clusterUuids, or hostUuids must be a non-empty list, or 'all' must be set to true" | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
compute/src/main/java/org/zstack/compute/allocator/HostAllocatorApiInterceptor.java
around lines 75 to 77, the error message contains a typo ("none-empty") and is
awkwardly phrased; update the argerr text to use "non-empty" and reword for
clarity (e.g., "At least one of zoneUuids, clusterUuids, or hostUuids must be a
non-empty list, or set all=true"). Ensure the error code
(ORG_ZSTACK_COMPUTE_ALLOCATOR_10034) and exception type remain unchanged.
Comment on lines
+96
to
103
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10003, "no Enabled hosts found in the [%s] candidate hosts having the hypervisor type [%s]", candidates.size(), spec.getHypervisorType()); | ||
| } else { | ||
| if (spec.getHypervisorType() != null) { | ||
| error = operr("no host having state=Enabled status=Connected hypervisorType=%s found", spec.getHypervisorType()); | ||
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10004, "no host having state=Enabled status=Connected hypervisorType=%s found", spec.getHypervisorType()); | ||
| } else { | ||
| error = operr("no host having state=Enabled status=Connected found"); | ||
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10005, "no host having state=Enabled status=Connected found"); | ||
| } | ||
| } |
There was a problem hiding this comment.
避免在 hypervisorType 为 null 时触发 NPE
这里 spec.getHypervisorType() 可能为 null。当前逻辑在进入该分支时会直接调用 isNoHypervisor(spec.getHypervisorType())，最终在 equals() 时抛出 NPE，导致本次改动新增的错误码路径也无法触达。建议先判空再调用辅助方法。
- } else if (isNoHypervisor(spec.getHypervisorType())) {
+ } else if (spec.getHypervisorType() != null && isNoHypervisor(spec.getHypervisorType())) {
error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10003,
"no Enabled hosts found in the [%s] candidate hosts having the hypervisor type [%s]",
candidates.size(), spec.getHypervisorType());
} else {
- if (spec.getHypervisorType() != null) {
+ if (spec.getHypervisorType() != null) {
error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10004,
"no host having state=Enabled status=Connected hypervisorType=%s found",
spec.getHypervisorType());
} else {
error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10005,
"no host having state=Enabled status=Connected found");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10003, "no Enabled hosts found in the [%s] candidate hosts having the hypervisor type [%s]", candidates.size(), spec.getHypervisorType()); | |
| } else { | |
| if (spec.getHypervisorType() != null) { | |
| error = operr("no host having state=Enabled status=Connected hypervisorType=%s found", spec.getHypervisorType()); | |
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10004, "no host having state=Enabled status=Connected hypervisorType=%s found", spec.getHypervisorType()); | |
| } else { | |
| error = operr("no host having state=Enabled status=Connected found"); | |
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10005, "no host having state=Enabled status=Connected found"); | |
| } | |
| } | |
| } else if (spec.getHypervisorType() != null && isNoHypervisor(spec.getHypervisorType())) { | |
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10003, | |
| "no Enabled hosts found in the [%s] candidate hosts having the hypervisor type [%s]", | |
| candidates.size(), spec.getHypervisorType()); | |
| } else { | |
| if (spec.getHypervisorType() != null) { | |
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10004, | |
| "no host having state=Enabled status=Connected hypervisorType=%s found", | |
| spec.getHypervisorType()); | |
| } else { | |
| error = operr(ORG_ZSTACK_COMPUTE_ALLOCATOR_10005, | |
| "no host having state=Enabled status=Connected found"); | |
| } | |
| } |
| completion.success(); | ||
| } catch (ClusterException e) { | ||
| completion.fail(inerr(e.getMessage())); | ||
| completion.fail(inerr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage())); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
将 inerr 改为 operr，保持“预删除校验失败”为业务/操作错误的语义
预删除扩展点抛出 ClusterException 属于可预期的业务校验失败，使用 inerr(...) 会把错误归类为内部错误，影响错误码分类与对外行为（例如 REST 错误类别、告警聚合）。建议改为 operr(...) 并保留全局错误码参数。
最小修复（保持现有结构）：
-import static org.zstack.core.Platform.inerr;
+import static org.zstack.core.Platform.operr;
@@
- completion.fail(inerr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));
+ completion.fail(operr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));可选增强（携带上下文并提前返回，便于排障；符合“尽早返回”与自解释原则）：
@@
- try {
- for (ClusterInventory cinv : cinvs) {
- extpEmitter.preDelete(cinv);
- }
- completion.success();
- } catch (ClusterException e) {
- completion.fail(operr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));
- }
+ for (ClusterInventory cinv : cinvs) {
+ try {
+ extpEmitter.preDelete(cinv);
+ } catch (ClusterException e) {
+ completion.fail(operr(
+ ORG_ZSTACK_COMPUTE_CLUSTER_10002,
+ String.format("preDelete failed for cluster[uuid:%s, name:%s]: %s",
+ cinv.getUuid(), cinv.getName(), e.getMessage())
+ ));
+ return;
+ }
+ }
+ completion.success();验证脚本（确认仓库内 CloudOperationsErrorCode 的常规用法与本修复一致）：
基于规范：异常处理需表达真实语义并对上游呈现友好的错误提示；同时尽量尽早返回，减少嵌套。As per coding guidelines.
🏁 Script executed:
#!/bin/bash
# 统计使用 CloudOperationsErrorCode 时与 operr/inerr 的搭配情况
rg -nP --type=java 'operr\s*\(\s*CloudOperationsErrorCode' -C1
rg -nP --type=java 'inerr\s*\(\s*CloudOperationsErrorCode' -C1
# 定位本错误码在其他位置的用法
rg -nP --type=java '\bORG_ZSTACK_COMPUTE_CLUSTER_10002\b' -n -C2Length of output: 1330
🏁 Script executed:
#!/bin/bash
# 搜索 inerr 与 operr 的静态导入
rg -nP 'import static org.zstack.core.Platform\.(inerr|operr)' -n -C0
# 统计 inerr 与 operr 的调用实例
rg -nP --type=java '\b(inerr|operr)\s*\(' -n -C1Length of output: 270720
改为 operr 以准确反映业务校验失败
-import static org.zstack.core.Platform.inerr;
+import static org.zstack.core.Platform.operr;
@@
- completion.fail(inerr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));
+ completion.fail(operr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| completion.fail(inerr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage())); | |
| ++ b/compute/src/main/java/org/zstack/compute/cluster/ClusterCascadeExtension.java | |
| @@ | |
| import static org.zstack.core.Platform.operr; | |
| @@ | |
| completion.fail(operr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage())); |
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/cluster/ClusterCascadeExtension.java
around line 121, the call uses
completion.fail(inerr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage())); change
it to completion.fail(operr(ORG_ZSTACK_COMPUTE_CLUSTER_10002, e.getMessage()));
so that a business validation failure is reported with operr instead of inerr,
keeping the same error code and message.
Comment on lines
+155
to
156
| completion.fail(operr(ORG_ZSTACK_DIRECTORY_10009, "fail to create directory, directories are up to four levels")); | ||
| return; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
错误信息与校验条件不一致：实际限制是“>3 拒绝”，提示却说“最多四层”
当前条件 split.length > 3 表示最多允许 3 段（自定义层级），而提示语为 “up to four levels”。为避免用户困惑，请二选一：
- 若真实上限为 3 层：仅修正提示文案；
- 若真实上限为 4 层：应将条件改为
> 4（此处仅建议，不在本变更行内修改）。
文案修正示例（更贴合当前条件）：
- completion.fail(operr(ORG_ZSTACK_DIRECTORY_10009, "fail to create directory, directories are up to four levels"));
+ completion.fail(operr(ORG_ZSTACK_DIRECTORY_10009, "failed to create directory: directories are up to three levels (excluding the implicit default directory)"));（遵循“自解释代码/避免误导性信息”。As per coding guidelines）
修正提示文案与校验逻辑不符
将
completion.fail(operr(ORG_ZSTACK_DIRECTORY_10009, "fail to create directory, directories are up to four levels"));替换为
completion.fail(operr(ORG_ZSTACK_DIRECTORY_10009, "failed to create directory: directories are up to three levels (excluding the implicit default directory)"));🤖 Prompt for AI Agents
In plugin/directory/src/main/java/org/zstack/directory/DirectoryManagerImpl.java
around lines 155 to 156, the error message text ("fail to create directory,
directories are up to four levels") does not match the validation logic which
enforces up to three levels (excluding the implicit default directory); update
the failure message to accurately reflect the validation by replacing it with a
message like "failed to create directory: directories are up to three levels
(excluding the implicit default directory)" so the log matches the enforced rule
and aids debugging.
Comment on lines
+186
to
187
| comp.fail(operr(ORG_ZSTACK_EXPON_10004, "not supported protocol[%s]", v.getProtocol())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
整体 LGTM：关键路径已切换至 CloudOperationsErrorCode
以上行号处的 operr(...) 变更与本 PR 目标一致。为确保遗漏清零，请执行下方脚本扫描剩余 RuntimeException 与“unsupport”拼写：
Also applies to: 465-466, 540-541, 577-578, 841-843, 992-993, 1014-1016, 1216-1219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java"
echo "Leftover RuntimeException throws:"
rg -n -C1 $'\\bthrow\\s+new\\s+RuntimeException\\b' "$FILE" || true
echo
echo "Typos of 'unsupport':"
rg -n -C1 'unsupport' "$FILE" || true
echo
echo "operr(...) call sites by code:"
rg -nP 'operr\\(ORG_ZSTACK_EXPON_\\d{5}' "$FILE" || trueLength of output: 2345
替换剩余 RuntimeException 并修正拼写
- 脚本扫描发现 ExponStorageController.java 中仍有多处
throw new RuntimeException(...)（如135–137、149–151、157–159、195–197、217–219、227–229、243–245、626–628、691–693、708–710、713–715、755–757、1155–1157），需统一改为comp.fail(operr(...))并使用对应的 CloudOperationsErrorCode。 - 修正第1156行
"unsupport target"为"unsupported target"。
🤖 Prompt for AI Agents
In plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
around lines 135–137, 149–151, 157–159, 186–187, 195–197, 217–219, 227–229,
243–245, 626–628, 691–693, 708–710, 713–715, 755–757 and 1155–1157 replace all
occurrences of throw new RuntimeException(...) with comp.fail(operr(...)) using
the appropriate CloudOperationsErrorCode constant for each error case (preserve
the original error message content as the opErr message parameters), and correct
the spelling at line 1156 from "unsupport target" to "unsupported target".
Comment on lines
+345
to
346
| throw new OperationFailureException(operr(ORG_ZSTACK_EXTERNALSTORAGE_PRIMARY_KVM_10001, "find active clients for volume[uuid:%s, installPath %s, client:%s]", | ||
| vol.getUuid(), vol.getInstallPath(), client.getManagerIp())); |
There was a problem hiding this comment.
错误文案细节：英文语法与占位符标点需微调
“find active clients …”不够自然，且 installPath %s 缺少冒号。建议改为更清晰的表述。
应用如下改动：
- throw new OperationFailureException(operr(ORG_ZSTACK_EXTERNALSTORAGE_PRIMARY_KVM_10001, "find active clients for volume[uuid:%s, installPath %s, client:%s]",
- vol.getUuid(), vol.getInstallPath(), client.getManagerIp()));
+ throw new OperationFailureException(operr(ORG_ZSTACK_EXTERNALSTORAGE_PRIMARY_KVM_10001,
+ "active client exists for volume[uuid:%s, installPath:%s, client:%s]",
+ vol.getUuid(), vol.getInstallPath(), client.getManagerIp()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new OperationFailureException(operr(ORG_ZSTACK_EXTERNALSTORAGE_PRIMARY_KVM_10001, "find active clients for volume[uuid:%s, installPath %s, client:%s]", | |
| vol.getUuid(), vol.getInstallPath(), client.getManagerIp())); | |
| throw new OperationFailureException(operr(ORG_ZSTACK_EXTERNALSTORAGE_PRIMARY_KVM_10001, | |
| "active client exists for volume[uuid:%s, installPath:%s, client:%s]", | |
| vol.getUuid(), vol.getInstallPath(), client.getManagerIp())); |
🤖 Prompt for AI Agents
In
plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java
around lines 345-346, the exception message "find active clients for
volume[uuid:%s, installPath %s, client:%s]" is grammatically awkward and missing
a colon after installPath; update the message to a clear English phrase and add
the colon in the placeholders — for example use "found active clients for volume
[uuid:%s, installPath:%s, client:%s]" or "failed to find active clients for
volume [uuid:%s, installPath:%s, client:%s]" depending on intent — keeping the
same parameters and error code.
| final T rsp = rep.toResponse(respType); | ||
| if (!rsp.success) { | ||
| completion.fail(operr("operation error, because:%s", rsp.error)); | ||
| completion.fail(operr(ORG_ZSTACK_ISCSI_KVM_10000, "operation error, because:%s", (String) rsp.error)); |
There was a problem hiding this comment.
错误信息英文表述不自然且缺少上下文；去掉不必要的类型强转
- 文案建议改为简洁自然的英文，并携带
hostUuid与path以便排障。 rsp.error通常为字符串，不需要(String)强转，避免潜在ClassCastException并提升可读性。
- completion.fail(operr(ORG_ZSTACK_ISCSI_KVM_10000, "operation error, because:%s", (String) rsp.error));
+ completion.fail(operr(
+ ORG_ZSTACK_ISCSI_KVM_10000,
+ "KVM agent HTTP call failed on host[%s], path[%s]: %s",
+ hostUuid, path, rsp.error));As per coding guidelines
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| completion.fail(operr(ORG_ZSTACK_ISCSI_KVM_10000, "operation error, because:%s", (String) rsp.error)); | |
| completion.fail(operr( | |
| ORG_ZSTACK_ISCSI_KVM_10000, | |
| "KVM agent HTTP call failed on host[%s], path[%s]: %s", | |
| hostUuid, path, rsp.error)); |
🤖 Prompt for AI Agents
In plugin/iscsi/src/main/java/org/zstack/iscsi/kvm/KvmIscsiNodeServer.java
around line 348, the error message is awkward and unnecessarily casts rsp.error
to (String); change to a concise, natural English message that includes hostUuid
and path for context, remove the (String) cast and pass rsp.error directly (or
use String.valueOf(rsp.error) if null-safety desired), and call
completion.fail(operr(ORG_ZSTACK_ISCSI_KVM_10000, "Failed to attach iSCSI node
on host %s for path %s: %s", hostUuid, path, rsp.error)) so the log is clear and
safe.
Comment on lines
81
to
85
| ErrorCode err = operr( | ||
| "failed to create bridge[%s] for l2Network[uuid:%s, type:%s] on kvm host[uuid:%s], because %s", cmd | ||
| ORG_ZSTACK_KVM_10155, "failed to create bridge[%s] for l2Network[uuid:%s, type:%s] on kvm host[uuid:%s], because %s", cmd | ||
| .getBridgeName(), l2Network.getUuid(), l2Network.getType(), hostUuid, rsp.getError()); | ||
| completion.fail(err); | ||
| return; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
创建桥失败路径：带全局错误码的 operr 使用正确
占位符与参数一一对应，错误上下文信息充分。请确认常量 ORG_ZSTACK_KVM_10155 的语义在 CloudOperationsErrorCode 中定义为 KVM/L2 无 VLAN 桥创建失败场景，避免语义复用。
🏁 Script executed:
#!/bin/bash
# 查找 CloudOperationsErrorCode 中对 10155 的定义与注释
rg -nC2 -P 'ORG_ZSTACK_KVM_10155\b|CloudOperationsErrorCode' --type javaLength of output: 234472
补充 ORG_ZSTACK_KVM_10155 常量的语义注释
CloudOperationsErrorCode.java 中 ORG_ZSTACK_KVM_10155 常量缺少对应场景说明（KVM/L2 无 VLAN 桥创建失败），请在常量定义处添加 Javadoc 注释以明确语义，避免后续复用混淆。
🤖 Prompt for AI Agents
In plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2NoVlanNetworkBackend.java
around lines 81 to 85, the error uses ORG_ZSTACK_KVM_10155 but the constant in
CloudOperationsErrorCode.java lacks a Javadoc describing the exact scenario;
update CloudOperationsErrorCode.java to add a clear Javadoc for
ORG_ZSTACK_KVM_10155 stating it represents failures to create bridges for KVM L2
no-VLAN networks (include context: bridge name, l2Network uuid/type and host
uuid) so future readers understand the precise semantics and avoid misuse.
Comment on lines
39
to
+41
| import static org.zstack.core.Platform.argerr; | ||
| import static org.zstack.core.Platform.operr; | ||
| import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*; |
There was a problem hiding this comment.
🧩 Analysis chain
全局巡检：是否仍有未携带全局错误码的 argerr/operr 用法
为了确保切换完全到“带全局错误码”的版本，建议在仓库内巡检还存在的纯文案 argerr/operr 调用。
Also applies to: 70-80, 101-114, 144-151, 248-252, 317-321, 413-417
🏁 Script executed:
#!/bin/bash
# 1) 列出所有 argerr/operr 调用点（供人工甄别是否均携带错误码常量）
rg -nP --type=java -C1 '\b(argerr|operr)\s*\('
# 2) 粗略过滤可能不带错误码的调用（启发式，需人工复核）
rg -nP --type=java -C1 '\b(argerr|operr)\s*\(\s*"(?:[^"]|\\")+' || trueLength of output: 280707
修复未携带全局错误码常量的 operr 调用
VolumeSnapshotApiInterceptor.java 第 134 行与第 143 行的 operr("…", …) 调用需替换为对应的全局错误码常量版本。
🤖 Prompt for AI Agents
In core/src/main/java/org/zstack/core/errorcode/ElaborationManagerImpl.java
around lines 39-41, ensure the static import of the global
CloudOperationsErrorCode constants is present (import static
org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*) and then update the
two operr(...) usages in VolumeSnapshotApiInterceptor.java (around lines 134 and
143) to call the operr variant that accepts a CloudOperationsErrorCode constant
as the first argument instead of only a formatted message; replace the plain
operr("…", …) calls with operr(<appropriate_GLOBAL_ERROR_CODE>, "descriptive
message", args) using the corresponding global error code constants and keep the
original messages/arguments.
Comment on lines
70
to
80
| private void preCheckElaborationContent(String filename, String jsonContent, ReturnValueCompletion<List<ElaborationCheckResult>> completion) { | ||
| if (filename == null && jsonContent == null) { | ||
| completion.fail(argerr("non file or jsoncontent input")); | ||
| completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10000, "non file or jsoncontent input")); | ||
| return; | ||
| } | ||
|
|
||
| if (filename != null && jsonContent != null) { | ||
| completion.fail(argerr("file or jsoncontent cannot both nonempty")); | ||
| completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10001, "file or jsoncontent cannot both nonempty")); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
输入校验错误文案英文不通顺，建议修正
保持专业英文可读性，并与参数名一致（jsonContent）。建议如下：
- completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10000, "non file or jsoncontent input"));
+ completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10000, "either 'filename' or 'jsonContent' must be provided"))
- completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10001, "file or jsoncontent cannot both nonempty"));
+ completion.fail(argerr(ORG_ZSTACK_CORE_ERRORCODE_10001, "'filename' and 'jsonContent' cannot both be set"))As per coding guidelines
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In core/src/main/java/org/zstack/core/errorcode/ElaborationManagerImpl.java
around lines 70 to 80, the input validation error messages are grammatically
incorrect and inconsistent with the parameter name jsonContent; update the
failure messages to be clear and professional English and reference the
jsonContent parameter explicitly (e.g., "no file or jsonContent provided" for
the null/null case and "file and jsonContent cannot both be non-empty" for the
both-nonnull case), preserving the same error codes and using
completion.fail(argerr(...)) as currently implemented.
Comment on lines
248
to
252
|
|
||
| if (!NumberUtils.isNumber(err.getCode())) { | ||
| trigger.fail(operr("elaboration code must be number!")); | ||
| trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10005, "elaboration code must be number!")); | ||
| return; | ||
| } |
There was a problem hiding this comment.
错误文案小语法问题
“must be number” → “must be a number.”，同时保持句号或不带感叹号的一致风格。
- trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10005, "elaboration code must be number!"));
+ trigger.fail(operr(ORG_ZSTACK_CORE_ERRORCODE_10005, "elaboration code must be a number."));As per coding guidelines
🤖 Prompt for AI Agents
In core/src/main/java/org/zstack/core/errorcode/ElaborationManagerImpl.java
around lines 248-252, the error message currently reads "elaboration code must
be number!" — update this literal to "elaboration code must be a number." (use
the article "a" and a period) and ensure punctuation style matches project
guidelines; apply the same phrasing style to any nearby similar messages if
present.
Comment on lines
413
to
417
|
|
||
|
|
||
| if (msg.getCategory() == null && msg.getRegex() == null){ | ||
| throw new OperationFailureException(Platform.argerr("input args 'regex' or 'category' must be set")); | ||
| throw new OperationFailureException(Platform.argerr(ORG_ZSTACK_CORE_ERRORCODE_10007, "input args 'regex' or 'category' must be set")); | ||
| } |
There was a problem hiding this comment.
API 入参提示英文建议更清晰
表达“至少需要一个参数被设置”更自然。
- throw new OperationFailureException(Platform.argerr(ORG_ZSTACK_CORE_ERRORCODE_10007, "input args 'regex' or 'category' must be set"));
+ throw new OperationFailureException(Platform.argerr(ORG_ZSTACK_CORE_ERRORCODE_10007, "either 'regex' or 'category' must be set"));As per coding guidelines
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (msg.getCategory() == null && msg.getRegex() == null){ | |
| throw new OperationFailureException(Platform.argerr("input args 'regex' or 'category' must be set")); | |
| throw new OperationFailureException(Platform.argerr(ORG_ZSTACK_CORE_ERRORCODE_10007, "input args 'regex' or 'category' must be set")); | |
| } | |
| if (msg.getCategory() == null && msg.getRegex() == null){ | |
| throw new OperationFailureException(Platform.argerr(ORG_ZSTACK_CORE_ERRORCODE_10007, "either 'regex' or 'category' must be set")); | |
| } |
🤖 Prompt for AI Agents
In core/src/main/java/org/zstack/core/errorcode/ElaborationManagerImpl.java
around lines 413 to 417, the thrown OperationFailureException message is
awkward; update the Platform.argerr call so the message reads a clearer English
phrase such as "at least one of 'regex' or 'category' must be set" (preserve
ORG_ZSTACK_CORE_ERRORCODE_10007 and argument formatting), replacing the current
"input args 'regex' or 'category' must be set" text.
Comment on lines
+128
to
131
| evt.setErrorCode(err(ORG_ZSTACK_CORE_JOB_10000, SysErrors.MANAGEMENT_NODE_UNAVAILABLE_ERROR, | ||
| "management node[id:%s] becomes unavailable, job[name:%s, id:%s] is not restartable", mgmtId, e.getName(), e.getId())); | ||
| bus.publish(evt); | ||
| logger.debug(String.format("[Job Removed]: job[id:%s, name:%s] because it's not restartable", |
There was a problem hiding this comment.
🧩 Analysis chain
发布 JobEvent 时未设置 jobId，监听方无法关联该 Job
handleEvent 通过 jobId 从 wrappers 中获取回调对象；此处未设置 jobId 会导致事件无法被正确路由。建议补充设置。
另外，请确认这里选择 err(...) 而非 operr(...) 是否是有意为之，以与全局错误码策略保持一致。
- JobEvent evt = new JobEvent();
- evt.setErrorCode(err(ORG_ZSTACK_CORE_JOB_10000, SysErrors.MANAGEMENT_NODE_UNAVAILABLE_ERROR,
+ JobEvent evt = new JobEvent();
+ evt.setJobId(e.getId());
+ evt.setErrorCode(err(ORG_ZSTACK_CORE_JOB_10000, SysErrors.MANAGEMENT_NODE_UNAVAILABLE_ERROR,
"management node[id:%s] becomes unavailable, job[name:%s, id:%s] is not restartable", mgmtId, e.getName(), e.getId()));
bus.publish(evt);🏁 Script executed:
#!/bin/bash
# 定位并打印 JobQueueFacadeImpl2.java 第120-140行内容
FILE=$(find . -path '*/core/src/main/java/org/zstack/core/job/JobQueueFacadeImpl2.java' | head -n1)
echo "FILE=$FILE"
sed -n '120,140p' "$FILE"Length of output: 1180
补充 JobEvent.setJobId 并确认错误类型
- 在发布事件前调用
evt.setJobId(e.getId())，否则监听方无法通过jobId将事件路由到对应的回调。 - 管理节点不可用属于系统级错误，使用
err(...)与全局错误码策略保持一致，无需改为operr(...)。
🤖 Prompt for AI Agents
In core/src/main/java/org/zstack/core/job/JobQueueFacadeImpl2.java around lines
128 to 131, before publishing the event you must set the job id on the event so
listeners can route callbacks (add evt.setJobId(e.getId()) prior to
bus.publish(evt)); also keep using the system-level error factory err(...) for
the management-node-unavailable case (do not change to operr(...)) so the global
error-code strategy remains consistent.
Comment on lines
128
to
131
| throw new ApiMessageInterceptionException( | ||
| argerr("Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) | ||
| argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
【致命：格式化占位符错误会在运行时抛异常】字符串 "valid values: [%,%s]" 中的 "%," 不是合法占位符，调用时会触发 UnknownFormatConversionException/MissingFormatArgumentException。请修复为两个 %s。
- argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE)
+ argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%s,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE)为了避免类似问题遗漏，建议在仓库中扫描可疑的 "%," 用法与该具体字符串：
🏁 Script executed:
#!/bin/bash
rg -nP --type=java '\[%,%s\]'
rg -nP --type=java '%,(?=[^\w%])'Length of output: 1086
修复所有格式化占位符错误
-
plugin/ldap/src/main/java/org/zstack/ldap/LdapApiInterceptor.java 第129行：将
"valid values: [%,%s]"修改为"valid values: [%s,%s]"- argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) + argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%s,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE)
-
conf/i18n/messages_en_US.properties & conf/i18n/messages_zh_CN.properties 第6303行：将键和值中所有
[%,%s]改为[%s,%s]，并在英文值中使用{1}、{2}占位符，在中文值中同步更新- Wrong\ LdapServerType[%s],\ valid\ values\:\ [%,%s] = Wrong LdapServerType[{0}], valid values: [%,{1}] + Wrong\ LdapServerType[%s],\ valid\ values\:\ [%s,%s] = Wrong LdapServerType[{0}], valid values: [{1},{2}]
-
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/portforwarding/ApplyPortforwardingRuleOnVirtualRouterVmFlow.java 第125行：将
"rules %,"修改为"rules %s,"- String err = String.format("failed to revoke port forwarding rules %, because %s", JSONObjectUtil.toJsonString(to), ret.getError()); + String err = String.format("failed to revoke port forwarding rules %s, because %s", JSONObjectUtil.toJsonString(to), ret.getError());
以上修改可防止 UnknownFormatConversionException 异常。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new ApiMessageInterceptionException( | |
| argerr("Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) | |
| argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%,%s]", type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) | |
| ); | |
| } | |
| throw new ApiMessageInterceptionException( | |
| argerr(ORG_ZSTACK_LDAP_10010, "Wrong LdapServerType[%s], valid values: [%s,%s]", | |
| type, LdapConstant.OpenLdap.TYPE, LdapConstant.WindowsAD.TYPE) | |
| ); |
🤖 Prompt for AI Agents
plugin/ldap/src/main/java/org/zstack/ldap/LdapApiInterceptor.java lines 128-131,
conf/i18n/messages_en_US.properties line 6303,
conf/i18n/messages_zh_CN.properties line 6303, and
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/portforwarding/ApplyPortforwardingRuleOnVirtualRouterVmFlow.java
line 125: Fix formatting placeholder mistakes that cause
UnknownFormatConversionException by replacing malformed placeholders—change
"valid values: [%,%s]" to "valid values: [%s,%s]" in LdapApiInterceptor, update
both keys and values in the two properties files replacing all occurrences of
"[%,%s]" with "[%s,%s]" and use {1},{2} style placeholders in the English value
and the corresponding numbered placeholders in the Chinese value, and change
"rules %," to "rules %s," in ApplyPortforwardingRuleOnVirtualRouterVmFlow so all
format strings use correct %s placeholders.
| logger.debug("Cannot connect to LDAP/AD server, Invalid Credentials, please checkout User DN and password", e); | ||
| return operr("Cannot connect to LDAP/AD server, Invalid Credentials, please checkout User DN and password"); | ||
| return operr(ORG_ZSTACK_LDAP_10012, "Cannot connect to LDAP/AD server, Invalid Credentials, please checkout User DN and password"); | ||
| } catch (CommunicationException e) { |
There was a problem hiding this comment.
【英文文案小问题】返回的错误消息里有用词问题：
- “Invalid Credentials” 大小写建议统一为 “Invalid credentials”。
- “please checkout …” 应为 “please check …” 或 “please check out …”。
- “communication false” 应为 “communication failure”。
- return operr(ORG_ZSTACK_LDAP_10012, "Cannot connect to LDAP/AD server, Invalid Credentials, please checkout User DN and password");
+ return operr(ORG_ZSTACK_LDAP_10012, "Cannot connect to LDAP/AD server. Invalid credentials. Please check the User DN and password.");
- return operr(ORG_ZSTACK_LDAP_10013, "Cannot connect to LDAP/AD server, communication false, please checkout IP, port and Base DN");
+ return operr(ORG_ZSTACK_LDAP_10013, "Cannot connect to LDAP/AD server. Communication failure. Please check the IP, port, and Base DN.");
- return operr(ORG_ZSTACK_LDAP_10014, "Cannot connect to LDAP/AD server, %s", e.toString());
+ return operr(ORG_ZSTACK_LDAP_10014, "Cannot connect to LDAP/AD server: %s", e.getMessage());Also applies to: 157-157, 161-161
🤖 Prompt for AI Agents
In plugin/ldap/src/main/java/org/zstack/ldap/LdapApiInterceptor.java around
lines 155-161, the error message text contains awkward wording and inconsistent
casing; update the thrown/returned messages to use "Invalid credentials"
(lowercase 'credentials'), replace "please checkout" with "please check" or
"please check out" consistently (use "please check" here), and change
"communication false" to "communication failure"; apply these wording changes to
the messages at lines 155, 157 and 161 so all three messages use the corrected,
consistent phrases.
| import static org.zstack.core.Platform.*; | ||
| import static org.zstack.utils.CollectionDSL.e; | ||
| import static org.zstack.utils.CollectionDSL.map; | ||
| import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*; |
There was a problem hiding this comment.
🧩 Analysis chain
校验本文件使用到的全局错误码是否已在枚举中定义且未重复
为避免运行期找不到错误码或与其他模块冲突，请批量校验这些符号在 CloudOperationsErrorCode 中存在且唯一：ORG_ZSTACK_NETWORK_SERVICE_LB_10017 ~ ..._10026。
Also applies to: 768-768, 774-774, 779-779, 855-855, 1179-1179, 1656-1656, 1685-1686, 1787-1788, 2173-2174, 2345-2346
🏁 Script executed:
#!/bin/bash
# 校验本文件引用的错误码定义是否存在且唯一
set -euo pipefail
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
for c in "${codes[@]}"; do
echo "Checking $c ..."
rg -nP --type=java "\b$c\b" -C1 | tee /dev/stderr | awk 'END{ if(NR==0) exit 2 }'
done
echo "All listed error codes are referenced somewhere in repo."
# 进一步定位定义位置（假设在 CloudOperationsErrorCode 或相关枚举中）
for c in "${codes[@]}"; do
echo "Definition candidate for $c:"
rg -nP --type=java "(enum|interface|class)\s+CloudOperationsErrorCode|$c" -C2 | sed 's/^/ /'
doneLength of output: 91
下面的脚本移除了对 tee 的依赖，并校验 CloudOperationsErrorCode 枚举文件中每个错误码是否定义且仅出现一次：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
# 定位枚举文件
enum_file=$(rg -l --type java "enum CloudOperationsErrorCode" -g '*.java')
echo "Found enum file: $enum_file"
# 验证每个错误码在枚举中仅定义一次
for c in "${codes[@]}"; do
echo -n "Verifying definition for $c ... "
occurrences=$(rg -o --type java "\b${c}\s*\(" "$enum_file" | wc -l)
if [ "$occurrences" -eq 1 ]; then
echo "OK"
else
echo "ERROR: Found $occurrences definitions for $c"
exit 1
fi
done
echo "All listed error codes are defined exactly once."Length of output: 0
下面的脚本将定位并打印 CloudOperationsErrorCode 定义所在文件的前 200 行，以便确认其类型（enum/class/interface）及内容，再逐一验证所有错误码是否在该文件中定义且唯一：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. 定位所有包含 CloudOperationsErrorCode 的 Java 文件
echo "查找包含 CloudOperationsErrorCode 的文件："
files=$(rg -l --type java "CloudOperationsErrorCode" -g '*.java' || true)
if [ -z "$files" ]; then
echo "未找到任何包含 CloudOperationsErrorCode 的文件，请手动确认文件名。"
exit 1
fi
echo "$files"
# 2. 打印每个文件的前 200 行，确认定义类型
for f in $files; do
echo
echo "==== $f (前 200 行) ===="
sed -n '1,200p' "$f"
done
# 3. 验证错误码定义（假设第一个文件是目标定义）
enum_file=$(echo "$files" | head -n1)
echo
echo "使用 $enum_file 校验错误码定义："
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
for c in "${codes[@]}"; do
echo -n "验证 $c ... "
count=$(rg -o "\b$c\b" "$enum_file" | wc -l)
if [ "$count" -eq 1 ]; then
echo "OK"
else
echo "ERROR: 在 $enum_file 中找到 $count 次"
exit 1
fi
done
echo "所有错误码在 $enum_file 中均已定义且唯一。"Length of output: 262723
下面脚本直接在 CloudOperationsErrorCode.java 中定位并打印每个错误码的定义行，以验证它们均已存在且唯一：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 定位枚举文件
enum_file=$(rg -l --type java 'interface CloudOperationsErrorCode' -g '*.java')
echo "使用枚举文件：$enum_file"
# 要校验的错误码
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
# 验证定义且唯一
for c in "${codes[@]}"; do
echo "验证 $c 定义："
rg -nP "\b$c\s*\(" "$enum_file"
doneLength of output: 0
下面脚本修正为查找 enum CloudOperationsErrorCode，并逐一统计每个错误码在该枚举文件中的出现次数，确保均定义且唯一：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 定位枚举文件
enum_file=$(rg -l --type java 'enum CloudOperationsErrorCode' -g '*.java')
echo "Found enum file: $enum_file"
# 要校验的错误码
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
for c in "${codes[@]}"; do
count=$(rg -o "\b${c}\b" "$enum_file" | wc -l)
echo "$c: $count"
if [ "$count" -ne 1 ]; then
echo "ERROR: $c 定义缺失或重复"
exit 1
fi
done
echo "所有错误码在 $enum_file 中均已定义且唯一。"Length of output: 0
下面脚本会：
- 定位
CloudOperationsErrorCode.java文件路径 - 打印该文件前 50 行，确认其定义形式
- 对每个错误码统计出现次数，确保定义且唯一
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. 定位文件
file=$(rg -l 'CloudOperationsErrorCode.java' -g '*.java')
echo "Found file: $file"
# 2. 打印前 50 行
echo "===== File header (first 50 lines) ====="
sed -n '1,50p' "$file"
# 3. 验证每个错误码出现次数
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
echo
echo "===== Error code counts ====="
for c in "${codes[@]}"; do
count=$(grep -o "\\b${c}\\b" "$file" | wc -l)
echo "${c}: ${count}"
if [ "$count" -ne 1 ]; then
echo "ERROR: ${c} 定义缺失或重复"
exit 1
fi
done
echo "所有错误码均已在 $file 中唯一定义。"Length of output: 0
下面脚本改用 find 定位 CloudOperationsErrorCode.java，并校验前 50 行及错误码定义出现次数：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. 定位文件
file=$(find . -type f -name CloudOperationsErrorCode.java | head -n1)
if [ -z "$file" ]; then
echo "ERROR: 未找到 CloudOperationsErrorCode.java，请手动确认路径"
exit 1
fi
echo "Found file: $file"
# 2. 打印前 50 行
echo "===== File header (first 50 lines) ====="
sed -n '1,50p' "$file"
echo
# 3. 验证每个错误码出现次数
codes=(
ORG_ZSTACK_NETWORK_SERVICE_LB_10017
ORG_ZSTACK_NETWORK_SERVICE_LB_10018
ORG_ZSTACK_NETWORK_SERVICE_LB_10019
ORG_ZSTACK_NETWORK_SERVICE_LB_10020
ORG_ZSTACK_NETWORK_SERVICE_LB_10021
ORG_ZSTACK_NETWORK_SERVICE_LB_10022
ORG_ZSTACK_NETWORK_SERVICE_LB_10023
ORG_ZSTACK_NETWORK_SERVICE_LB_10024
ORG_ZSTACK_NETWORK_SERVICE_LB_10025
ORG_ZSTACK_NETWORK_SERVICE_LB_10026
)
echo "===== Error code counts ====="
for c in "${codes[@]}"; do
count=$(grep -o "\\b${c}\\b" "$file" | wc -l)
echo "${c}: ${count}"
if [ "$count" -ne 1 ]; then
echo "ERROR: ${c} 定义缺失或重复"
exit 1
fi
done
echo "所有错误码均在 $file 中唯一定义。"Length of output: 2851
修复重复的全局错误码并完成校验
在 utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java 中发现 ORG_ZSTACK_NETWORK_SERVICE_LB_10017 重复定义（出现 2 次）；移除或合并重复项，使该常量仅定义一次。脚本在遇到此重复后中止；修复后重新运行验证，确保 ORG_ZSTACK_NETWORK_SERVICE_LB_10017..ORG_ZSTACK_NETWORK_SERVICE_LB_10026 在该文件中均存在且唯一。
| VipVO vipVO = Q.New(VipVO.class).eq(VipVO_.uuid, msg.getVipUuid()).find(); | ||
| if (StringUtils.isEmpty(vipVO.getIp())) { | ||
| throw new OperationFailureException(operr("fail to attach vip to lb , because vip[%s] has no ip", vipVO.getUuid())); | ||
| throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10017, "fail to attach vip to lb , because vip[%s] has no ip", vipVO.getUuid())); |
There was a problem hiding this comment.
修复与规范化 Attach VIP 的报错文本；同时纠正 IPv6 分支中错误的 UUID 参数
- Line 779 传参错误：应展示已存在的 IPv6 VIP（
getIpv6VipUuid()），否则会误导定位。此为功能性缺陷（错误信息不准确）。 - 其余两处建议统一英文与术语、去掉多余空格，符合“英文无拼写错误”的规范。
请应用以下补丁：
-throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10017, "fail to attach vip to lb , because vip[%s] has no ip", vipVO.getUuid()));
+throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10017,
+ "failed to attach VIP to load balancer: VIP[%s] has no IP", vipVO.getUuid()));
-throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10018, "fail to attach ipv4 vip to lb , because lb[%s] has ipv4 vip[%s]", self.getUuid(), self.getVipUuid()));
+throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10018,
+ "failed to attach IPv4 VIP: load balancer[uuid:%s] already has IPv4 VIP[%s]",
+ self.getUuid(), self.getVipUuid()));
-throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10019, "fail to attach ipv6 vip to lb , because lb[%s] has ipv6 vip[%s]", self.getUuid(), self.getVipUuid()));
+throw new OperationFailureException(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10019,
+ "failed to attach IPv6 VIP: load balancer[uuid:%s] already has IPv6 VIP[%s]",
+ self.getUuid(), self.getIpv6VipUuid()));基于代码规范
Also applies to: 774-774, 779-779
🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java
around lines 768, 774 and 779, the error messages for failing to attach a VIP
should be normalized and the IPv6 branch currently passes the wrong UUID; update
the messages to consistent, idiomatic English without extra spaces and ensure
the IPv6 case uses vipVO.getIpv6VipUuid() (not getUuid()), and update the other
two similar lines to the same formatted message "failed to attach VIP to load
balancer: vip[%s] has no IP" (or equivalent consistent wording) so logs are
accurate and uniform.
| if (bkd != null) { | ||
| if (!bkd.canDetachVipFromLb(lbStruct, vipVO)) { | ||
| reply.setError(operr("cloud not delete vip[%s]", vipVO.getUuid())); | ||
| reply.setError(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10020, "cloud not delete vip[%s]", vipVO.getUuid())); |
There was a problem hiding this comment.
拼写错误：cloud → could；并建议语义更贴近上下文
当前文案为 “cloud not delete vip”，应为 “could not detach VIP[...]”（此处是 detach 流程）。请修正以避免误导用户与违反英文规范。
-reply.setError(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10020, "cloud not delete vip[%s]", vipVO.getUuid()));
+reply.setError(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10020, "could not detach VIP[%s]", vipVO.getUuid()));基于代码规范
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| reply.setError(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10020, "cloud not delete vip[%s]", vipVO.getUuid())); | |
| reply.setError(operr(ORG_ZSTACK_NETWORK_SERVICE_LB_10020, "could not detach VIP[%s]", vipVO.getUuid())); |
🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.java
around line 855, the error message mistakenly uses "cloud not delete vip[%s]";
change this to a grammatically correct and context-appropriate message such as
"could not detach VIP[%s]" (keeping the same error code constant
ORG_ZSTACK_NETWORK_SERVICE_LB_10020 and formatting with vipVO.getUuid()), so the
log reflects the detach flow and correct English.
Comment on lines
71
to
76
| String name = spec.getImage().getName(); | ||
| throw new OperationFailureException(operr( | ||
| "The image[uuid:%s, name:%s] is on the backup storage[uuid:%s, type:%s] that requires to work with primary storage[types:%s]," + | ||
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10016, "The image[uuid:%s, name:%s] is on the backup storage[uuid:%s, type:%s] that requires to work with primary storage[types:%s]," + | ||
| "however, no host found suitable to work with those primary storage", spec.getImage().getUuid(), name, | ||
| spec.getRequiredBackupStorageUuid(),spec.getImage().getType(), possiblePrimaryStorageTypes | ||
| )); |
There was a problem hiding this comment.
错误信息参数不匹配：type 占位符传入了 image type
此处消息模版中的 “type:%s” 表达的是备份存储类型，应使用上文查询得到的 backup storage type 变量 type，当前传入了 spec.getImage().getType()（镜像类型），会误导排障。
- spec.getRequiredBackupStorageUuid(),spec.getImage().getType(), possiblePrimaryStorageTypes
+ spec.getRequiredBackupStorageUuid(), type, possiblePrimaryStorageTypes请一并检查同文件其它分支的占位符顺序与参数类型。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String name = spec.getImage().getName(); | |
| throw new OperationFailureException(operr( | |
| "The image[uuid:%s, name:%s] is on the backup storage[uuid:%s, type:%s] that requires to work with primary storage[types:%s]," + | |
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10016, "The image[uuid:%s, name:%s] is on the backup storage[uuid:%s, type:%s] that requires to work with primary storage[types:%s]," + | |
| "however, no host found suitable to work with those primary storage", spec.getImage().getUuid(), name, | |
| spec.getRequiredBackupStorageUuid(),spec.getImage().getType(), possiblePrimaryStorageTypes | |
| )); | |
| String name = spec.getImage().getName(); | |
| throw new OperationFailureException(operr( | |
| ORG_ZSTACK_COMPUTE_ALLOCATOR_10016, "The image[uuid:%s, name:%s] is on the backup storage[uuid:%s, type:%s] that requires to work with primary storage[types:%s]," + | |
| "however, no host found suitable to work with those primary storage", spec.getImage().getUuid(), name, | |
| spec.getRequiredBackupStorageUuid(), type, possiblePrimaryStorageTypes | |
| )); |
🤖 Prompt for AI Agents
In
compute/src/main/java/org/zstack/compute/allocator/BackupStorageSelectPrimaryStorageAllocatorFlow.java
around lines 71-76, the error message uses the placeholder "type:%s" but passes
spec.getImage().getType() (image type) instead of the backup storage type
variable `type`; replace the incorrect argument with the backup storage type
variable so the placeholders match the intended values and update the argument
order accordingly; also scan other branches in the same file for similar
placeholder/argument mismatches and correct them so each "%s" corresponds to the
correct variable type and order.
| import org.zstack.utils.path.PathUtil; | ||
|
|
||
| import static org.zstack.core.Platform.operr; | ||
| import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
校验错误码唯一性与覆盖率（仓内冲突/遗漏）
为避免冲突，建议扫仓确保 ORG_ZSTACK_COMPUTE_HOST_10000..10010 未被其他模块复用，并补充用例覆盖失败路径。
可用脚本：
🏁 Script executed:
#!/bin/bash
# 查找本文件使用的 Host 错误码是否被重复使用
rg -nP 'ORG_ZSTACK_COMPUTE_HOST_1000[0-9]'Length of output: 3846
经扫描确认 ORG_ZSTACK_COMPUTE_HOST_10000～10009 未被其他模块复用，请补充 HostIpmiPowerExecutor 的失败场景单元测试覆盖。
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/HostIpmiPowerExecutor.java
around line 22, there are CloudOperationsErrorCode values
ORG_ZSTACK_COMPUTE_HOST_10000～10009 that currently lack unit test coverage; add
unit tests that assert the executor returns/throws the corresponding error codes
in each failure scenario. Create a new test class under compute/src/test/java
(e.g., HostIpmiPowerExecutorFailureTest) that mocks external dependencies
(network/IPMI client, host inventory, config) to simulate each distinct failure
case (authentication failure, command timeout, connection refused, unsupported
operation, invalid parameters, response parse error, permission denied, device
busy, internal error, and unexpected exception) and verify the executor produces
the exact CloudOperationsErrorCode and message for each case; use parameterized
tests or one test per code, assert both error code and relevant context fields,
and clean up mocks between tests.
| if (CoreGlobalProperty.UNIT_TEST_ON) { | ||
| if (mockFail) { | ||
| return operr("mock power off host[%s] by ipmi failed.", host.getUuid()); | ||
| return operr(ORG_ZSTACK_COMPUTE_HOST_10000, "mock power off host[%s] by ipmi failed.", host.getUuid()); |
There was a problem hiding this comment.
错误消息一致性与可观测性提升（统一英文、去除嵌套 String.format、补充上下文）
- 统一 IPMI 文案与大小写；避免 “.because”“can not”等语法问题。
- 避免在 operr 内部再包一层 String.format，直接使用占位符与 varargs。
- 对“unexpected output / 连接失败 / 不可达”类错误，补充 stdout/stderr 与 retCode 以便排障。
建议补丁（逐段）：
- return operr(ORG_ZSTACK_COMPUTE_HOST_10000, "mock power off host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10000, "Mock IPMI power-off on host[%s] failed.", host.getUuid());- return operr(ORG_ZSTACK_COMPUTE_HOST_10001, "power off host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10001, "IPMI power-off on host[%s] failed.", host.getUuid());- return operr(ORG_ZSTACK_COMPUTE_HOST_10002, "mock power on host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10002, "Mock IPMI power-on on host[%s] failed.", host.getUuid());- return operr(ORG_ZSTACK_COMPUTE_HOST_10003, "power on host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10003, "IPMI power-on on host[%s] failed.", host.getUuid());- ErrorCode err = operr(ORG_ZSTACK_COMPUTE_HOST_10004, String.format("reboot host[%s:%d] failed. because host is already powered off",
- host.getIpmi().getIpmiAddress(), host.getIpmi().getIpmiPort()));
+ ErrorCode err = operr(ORG_ZSTACK_COMPUTE_HOST_10004,
+ "Reboot host[%s:%d] failed because the host is already powered off.",
+ host.getIpmi().getIpmiAddress(), host.getIpmi().getIpmiPort());- return operr(ORG_ZSTACK_COMPUTE_HOST_10005, "mock power reset host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10005, "Mock IPMI power reset on host[%s] failed.", host.getUuid());- return operr(ORG_ZSTACK_COMPUTE_HOST_10006, "power reset host[%s] by ipmi failed.", host.getUuid());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10006, "IPMI power reset on host[%s] failed.", host.getUuid());- return new Pair(HostPowerStatus.UN_CONFIGURED,
- operr(ORG_ZSTACK_COMPUTE_HOST_10007, "ipmi information is not complete."));
+ return new Pair<>(HostPowerStatus.UN_CONFIGURED,
+ operr(ORG_ZSTACK_COMPUTE_HOST_10007, "IPMI information is incomplete."));- return new Pair(HostPowerStatus.POWER_UNKNOWN, operr(ORG_ZSTACK_COMPUTE_HOST_10008, "host[%s] got unexpected return value", ipmi.getUuid()));
+ return new Pair<>(HostPowerStatus.POWER_UNKNOWN,
+ operr(ORG_ZSTACK_COMPUTE_HOST_10008, "Host[%s] returned unexpected output: %s",
+ ipmi.getUuid(), rst.getStdout().trim()));- return new Pair(HostPowerStatus.POWER_UNKNOWN, operr(ORG_ZSTACK_COMPUTE_HOST_10009, "host[%s] can not connect ipmi[%s], because:%s",
- ipmi.getUuid(),
- ipmi.getIpmiAddress(),
- rst.getStderr()));
+ return new Pair<>(HostPowerStatus.POWER_UNKNOWN,
+ operr(ORG_ZSTACK_COMPUTE_HOST_10009, "Cannot connect to IPMI[%s] for host[%s] (ret=%d): %s",
+ ipmi.getIpmiAddress(), ipmi.getUuid(), rst.getRetCode(), rst.getStderr()));- return operr(ORG_ZSTACK_COMPUTE_HOST_10010, "host ipmi[%s] is not reachable.because %s",
- ipmi.getIpmiAddress(),
- rst.getStderr());
+ return operr(ORG_ZSTACK_COMPUTE_HOST_10010, "IPMI[%s] is not reachable (ret=%d): %s",
+ ipmi.getIpmiAddress(), rst.getRetCode(), rst.getStderr());Also applies to: 79-80, 121-122, 131-132, 138-140, 161-162, 171-172, 193-193, 202-203, 205-209, 223-226
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/HostIpmiPowerExecutor.java
around lines 63, 79-80, 121-122, 131-132, 138-140, 161-162, 171-172, 193,
202-203, 205-209 and 223-226: standardize all IPMI error messages to a
consistent English style and capitalization (use "IPMI" and plain phrasing like
"IPMI power off failed"), remove nested String.format calls and pass
host.getUuid() and other values as varargs to operr (use operr(code, "message
with %s placeholders", arg1, arg2)), and for errors involving unexpected output,
connection failures or unreachable hosts include stdout, stderr and return code
in the operr message (e.g. add placeholders for stdout/stderr/retCode and pass
their variables) so logs contain full context for troubleshooting.
Comment on lines
+45
to
46
| throw new FlowException(err(ORG_ZSTACK_COMPUTE_VM_10034, VmErrors.ALLOCATE_MAC_ERROR, "unable to find an available mac address after re-try 5 times, too many collisions")); | ||
| } |
There was a problem hiding this comment.
报错英文措辞改进（拼写/语法）
“re-try” 建议改为 “retries”，并统一 “MAC”的大小写。
- throw new FlowException(err(ORG_ZSTACK_COMPUTE_VM_10034, VmErrors.ALLOCATE_MAC_ERROR, "unable to find an available mac address after re-try 5 times, too many collisions"));
+ throw new FlowException(err(ORG_ZSTACK_COMPUTE_VM_10034, VmErrors.ALLOCATE_MAC_ERROR, "unable to find an available MAC address after 5 retries; too many collisions"));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new FlowException(err(ORG_ZSTACK_COMPUTE_VM_10034, VmErrors.ALLOCATE_MAC_ERROR, "unable to find an available mac address after re-try 5 times, too many collisions")); | |
| } | |
| throw new FlowException(err(ORG_ZSTACK_COMPUTE_VM_10034, | |
| VmErrors.ALLOCATE_MAC_ERROR, | |
| "unable to find an available MAC address after 5 retries; too many collisions")); |
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/vm/TfVmNicFactory.java around lines
45-46, the exception message uses "re-try 5 times" and lowercase "mac"; change
the wording to correct grammar and capitalization by using "after 5 retries" (or
"after 5 attempts") and "MAC" instead of "mac" so the message reads e.g. "unable
to find an available MAC address after 5 retries, too many collisions".
Comment on lines
72
to
75
| if (!CollectionUtils.isEmpty(requirdPsUuids) && Collections.disjoint(requirdPsUuids, cachedPsUuids)) { | ||
| trigger.fail(operr("creation rely on image cache[uuid:%s, locate ps uuids: [%s]], cannot create other places.", imageUuid, cachedPsUuids)); | ||
| trigger.fail(operr(ORG_ZSTACK_COMPUTE_VM_10024, "creation rely on image cache[uuid:%s, locate ps uuids: [%s]], cannot create other places.", imageUuid, cachedPsUuids)); | ||
| return; | ||
| } else if (!CollectionUtils.isEmpty(requirdPsUuids)) { |
There was a problem hiding this comment.
英文报错语法优化
建议提升可读性（遵循“代码与报错为正确英文”的规范）。As per coding guidelines
- trigger.fail(operr(ORG_ZSTACK_COMPUTE_VM_10024, "creation rely on image cache[uuid:%s, locate ps uuids: [%s]], cannot create other places.", imageUuid, cachedPsUuids));
+ trigger.fail(operr(ORG_ZSTACK_COMPUTE_VM_10024,
+ "Creation relies on image cache [uuid:%s, located on primary storages: %s]; cannot create in other places.",
+ imageUuid, cachedPsUuids));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!CollectionUtils.isEmpty(requirdPsUuids) && Collections.disjoint(requirdPsUuids, cachedPsUuids)) { | |
| trigger.fail(operr("creation rely on image cache[uuid:%s, locate ps uuids: [%s]], cannot create other places.", imageUuid, cachedPsUuids)); | |
| trigger.fail(operr(ORG_ZSTACK_COMPUTE_VM_10024, "creation rely on image cache[uuid:%s, locate ps uuids: [%s]], cannot create other places.", imageUuid, cachedPsUuids)); | |
| return; | |
| } else if (!CollectionUtils.isEmpty(requirdPsUuids)) { | |
| if (!CollectionUtils.isEmpty(requirdPsUuids) && Collections.disjoint(requirdPsUuids, cachedPsUuids)) { | |
| trigger.fail(operr(ORG_ZSTACK_COMPUTE_VM_10024, | |
| "Creation relies on image cache [uuid:%s, located on primary storages: %s]; cannot create in other places.", | |
| imageUuid, cachedPsUuids)); | |
| return; | |
| } else if (!CollectionUtils.isEmpty(requirdPsUuids)) { |
🤖 Prompt for AI Agents
In
compute/src/main/java/org/zstack/compute/vm/VmAllocateHostAndPrimaryStorageFlow.java
around lines 72-75, the error message "creation rely on image cache[uuid:%s,
locate ps uuids: [%s]], cannot create other places." is grammatically incorrect
and unclear; update the trigger.fail(operr(...)) call to use a clear, proper
English message such as: "Creation depends on image cache (image UUID: %s)
located on primary storages: %s; cannot create VM on other locations." Ensure
punctuation and capitalization are fixed, keep the same placeholders/order, and
preserve the error code constant.
| if (LoadBalancerSystemTags.SESSION_PERSISTENCE.isMatch(tag) || LoadBalancerSystemTags.SESSION_IDLE_TIMEOUT.isMatch(tag) || LoadBalancerSystemTags.COOKIE_NAME.isMatch(tag)) { | ||
| throw new ApiMessageInterceptionException(argerr("l4[%s] loadBalancer listener[%s] doesn't support assigning session persistence state", msg.getProtocol(), msg.getName())); | ||
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10066, "l4[%s] loadBalancer listener[%s] doesn't support assigning session persistence state", msg.getProtocol(), msg.getName())); | ||
| } |
There was a problem hiding this comment.
可能的空指针风险：直接遍历 msg.getSystemTags()
多处直接 for (String tag : msg.getSystemTags())，当 systemTags == null 时会 NPE。建议统一做空集合兜底或前置空判断。
- if (LoadBalancerConstants.LB_PROTOCOL_UDP.equals(msg.getProtocol()) || LoadBalancerConstants.LB_PROTOCOL_TCP.equals(msg.getProtocol())) {
- for (String tag : msg.getSystemTags()) {
+ if (LoadBalancerConstants.LB_PROTOCOL_UDP.equals(msg.getProtocol()) || LoadBalancerConstants.LB_PROTOCOL_TCP.equals(msg.getProtocol())) {
+ if (msg.getSystemTags() != null) for (String tag : msg.getSystemTags()) {
if (LoadBalancerSystemTags.SESSION_PERSISTENCE.isMatch(tag) || LoadBalancerSystemTags.SESSION_IDLE_TIMEOUT.isMatch(tag) || LoadBalancerSystemTags.COOKIE_NAME.isMatch(tag)) {
throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10066, "l4[%s] loadBalancer listener[%s] doesn't support assigning session persistence state", msg.getProtocol(), msg.getName()));
}
}
}请在本方法内其它遍历 systemTags 的循环一并套用同样保护。
Also applies to: 901-906, 918-921, 1017-1025
Comment on lines
+996
to
997
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10083, "loadBalancer[%s] listener[%s] doesn't support assigning session persistence idle timeout without assigning rewrite mode", msg.getLoadBalancerUuid(), msg.getName())); | ||
| } |
There was a problem hiding this comment.
文案与逻辑矛盾（rewrite 模式下不允许 idle timeout）
此处判断为 “当启用 rewrite 且指定了 timeout → 不允许”，而当前文案写成 “without assigning rewrite mode”。应改为 “当模式为 rewrite 时不支持 idle timeout”。
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10083, "loadBalancer[%s] listener[%s] doesn't support assigning session persistence idle timeout without assigning rewrite mode", msg.getLoadBalancerUuid(), msg.getName()));
+ throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10083, "loadBalancer[%s] listener[%s] doesn't support assigning session persistence idle timeout when the mode is rewrite", msg.getLoadBalancerUuid(), msg.getName()));🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java
around lines 996-997, the thrown ApiMessageInterceptionException's message
contradicts the logic (logic blocks specifying idle timeout when mode is
rewrite), so update the error text to reflect that idle timeout is not supported
when the listener is in rewrite mode (e.g., change to "loadBalancer[%s]
listener[%s] does not support assigning session persistence idle timeout when
rewrite mode is enabled"), keeping the same args and error code.
Comment on lines
+1347
to
1349
| throw new OperationFailureException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10119, "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default", target)); | ||
| } | ||
|
|
There was a problem hiding this comment.
拦截器中抛出异常类型不一致（应统一为 ApiMessageInterceptionException）
这里直接抛出了 OperationFailureException，该分支不在本方法此前用于转换的 try/catch 内部，最终会以错误的异常类型向上冒泡。按照项目约定与既有学习，应在拦截阶段统一抛出 ApiMessageInterceptionException。
- throw new OperationFailureException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10119, "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default", target));
+ throw new ApiMessageInterceptionException(argerr(
+ ORG_ZSTACK_NETWORK_SERVICE_LB_10119,
+ "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default",
+ target
+ ));Based on learnings
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new OperationFailureException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10119, "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default", target)); | |
| } | |
| // ... | |
| if (!isValidHealthTarget(target)) { | |
| - throw new OperationFailureException(argerr( | |
| - ORG_ZSTACK_NETWORK_SERVICE_LB_10119, | |
| - "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default", | |
| - target | |
| throw new ApiMessageInterceptionException(argerr( | |
| ORG_ZSTACK_NETWORK_SERVICE_LB_10119, | |
| "invalid health target[%s], the format is targetCheckProtocol:port, for example, tcp:default", | |
| target | |
| )); | |
| } | |
| // ... |
🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java
around lines 1347-1349, the code currently throws OperationFailureException for
invalid health target which bypasses the interceptor's expected exception flow;
replace that throw with an ApiMessageInterceptionException carrying the same
argerr(...) message so interception-phase errors are consistently propagated as
ApiMessageInterceptionException (align with the method's existing try/catch and
project conventions).
Comment on lines
+1816
to
1817
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10160, "could not update backend server vmnic of serverGroup,because serverGroup[uuid:%s] don not have vmnic [uuid:%s] ",msg.getServerGroupUuid(),vmNic.containsKey("uuid"))); | ||
| } |
There was a problem hiding this comment.
错误变量用于日志占位符 + 英文错误（don not）
此处应打印 vmnic 的 uuid，但代码传入了 vmNic.containsKey("uuid")（布尔值）；英语也有拼写错误。修正为取值并优化文案。
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10160, "could not update backend server vmnic of serverGroup,because serverGroup[uuid:%s] don not have vmnic [uuid:%s] ",msg.getServerGroupUuid(),vmNic.containsKey("uuid")));
+ throw new ApiMessageInterceptionException(argerr(
+ ORG_ZSTACK_NETWORK_SERVICE_LB_10160,
+ "could not update backend server vmnic of server group, because serverGroup [uuid:%s] does not have vmnic [uuid:%s]",
+ msg.getServerGroupUuid(), vmNic.get("uuid")
+ ));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10160, "could not update backend server vmnic of serverGroup,because serverGroup[uuid:%s] don not have vmnic [uuid:%s] ",msg.getServerGroupUuid(),vmNic.containsKey("uuid"))); | |
| } | |
| throw new ApiMessageInterceptionException(argerr( | |
| ORG_ZSTACK_NETWORK_SERVICE_LB_10160, | |
| "could not update backend server vmnic of server group, because serverGroup [uuid:%s] does not have vmnic [uuid:%s]", | |
| msg.getServerGroupUuid(), vmNic.get("uuid") | |
| )); |
🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java
around lines 1816-1817, the exception message currently passes
vmNic.containsKey("uuid") (a boolean) and has a typo "don not"; replace the
boolean with the actual vmnic UUID value (e.g., obtain vmNic.get("uuid") into a
local String and pass that) and correct the wording to proper English ("does not
have vmnic") so the argerr call receives the serverGroup UUID and the vmnic UUID
values in the correct order; also handle possible null (format safely, e.g.,
String.valueOf(...)) before passing to argerr.
Comment on lines
+1873
to
1874
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10167, "could not add backend server ip to serverGroup[uuid:%s] ,because vmnic weight[%s] not a correct number",server.get("weight"))); | ||
| } |
There was a problem hiding this comment.
错误对象名（server ip 权重报错文案写成 vmnic）
这里属于后端 Server IP 的权重校验，文案应为 “server ip weight”。
- throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10167, "could not add backend server ip to serverGroup[uuid:%s] ,because vmnic weight[%s] not a correct number",server.get("weight")));
+ throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10167, "could not add backend server ip to serverGroup [uuid:%s], because server ip weight[%s] is not a valid number", msg.getServerGroupUuid(), server.get("weight")));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SERVICE_LB_10167, "could not add backend server ip to serverGroup[uuid:%s] ,because vmnic weight[%s] not a correct number",server.get("weight"))); | |
| } | |
| throw new ApiMessageInterceptionException(argerr( | |
| ORG_ZSTACK_NETWORK_SERVICE_LB_10167, | |
| "could not add backend server ip to serverGroup [uuid:%s], because server ip weight[%s] is not a valid number", | |
| msg.getServerGroupUuid(), | |
| server.get("weight") | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java
around lines 1873-1874, the error message uses "vmnic weight" when validating a
backend server IP's weight; update the message to read "server ip weight" (e.g.
replace "vmnic weight" with "server ip weight") so the exception text correctly
reflects that this is a server IP weight validation error.
Resolves: ZSTAC-66343 Change-Id: I6c737a7163706e72766764757a70787479746f77
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: ZSTAC-66343
Change-Id: I61787263626f646b6a6b6c6262666a6962787866
sync from gitlab !8487