improved errorlog message by adding null check#13263
improved errorlog message by adding null check#13263saurabhkushwaha438 wants to merge 2 commits into
Conversation
DaanHoogland
requested review from
Copilot and
nvazquez
and removed request for
nvazquez
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13263 +/- ##
============================================
- Coverage 18.11% 18.11% -0.01%
+ Complexity 16757 16749 -8
============================================
Files 6037 6037
Lines 542783 543025 +242
Branches 66454 66478 +24
============================================
+ Hits 98301 98342 +41
- Misses 433438 433625 +187
- Partials 11044 11058 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR prevents secondary NullPointerExceptions from masking the original VM import/conversion failure during VMware-to-KVM migration, so the UI/API can surface the real error (e.g., missing virt-v2v / nbdkit-vddk) as described in issue #13007.
Changes:
- Add a null guard before calling
updateImportVMTaskErrorState(...)from the VM import failure handler. - Add an early return in
updateImportVMTaskErrorState(...)when the provided task is null.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java | Avoids triggering a secondary NPE while handling a CloudRuntimeException during import. |
| server/src/main/java/org/apache/cloudstack/vm/ImportVmTasksManagerImpl.java | Makes task error-state updates resilient to null task objects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @Override | ||
| public void updateImportVMTaskErrorState(ImportVmTask importVMTask, ImportVmTask.TaskState state, String errorMsg) { | ||
| if (importVMTask == null) { |
saurabhkushwaha438
force-pushed
the
improved-errorlog-message
branch
from
afeae99 to
5f54d85
Compare
| logger.debug("Creating import VM task entry for VM: {} for account {} on zone {} " + | ||
| "from the vCenter: {} / datacenter: {} / source VM: {}", | ||
| "from the vCenter: {} / datacenter: {} / source VM: {}", | ||
| sourceVMName, owner.getAccountName(), zone.getName(), displayName, vcenter, datacenterName); |
There was a problem hiding this comment.
this is not actual change that i made , it is just a formatting to pass CI build because maven build enforces strict checkstyle rules.
There was a problem hiding this comment.
| sourceVMName, owner.getAccountName(), zone.getName(), displayName, vcenter, datacenterName); | |
| sourceVMName, owner.getAccountName(), zone.getName(), vcenter, datacenterName, displayName); |
There was a problem hiding this comment.
don’t worry about it @saurabhkushwaha438 . you can apply the suggestion (mine is better than co-pilot’s ;) ) or just resolve.
| importVMTaskVO.getDisplayName(), | ||
| importVMTaskVO.getVcenter(), importVMTaskVO.getDatacenter(), step); |
| } catch (CloudRuntimeException e) { | ||
| logger.error(String.format("Error importing VM: %s", e.getMessage()), e); | ||
| importVmTasksManager.updateImportVMTaskErrorState(importVMTask, ImportVmTask.TaskState.Failed, e.getMessage()); | ||
| ActionEventUtils.onCompletedActionEvent(userId, owner.getId(), EventVO.LEVEL_ERROR, EventTypes.EVENT_VM_IMPORT, | ||
| cmd.getEventDescription(), null, null, 0); | ||
| importVmTasksManager.updateImportVMTaskErrorState(importVMTask, ImportVmTask.TaskState.Failed, | ||
| e.getMessage()); | ||
| ActionEventUtils.onCompletedActionEvent(userId, owner.getId(), EventVO.LEVEL_ERROR, | ||
| EventTypes.EVENT_VM_IMPORT, cmd.getEventDescription(), null, null, 0); | ||
| throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage()); |
Implemented defensive null checks to prevent secondary NullPointerExceptions from masking the original VM import failure reason during VMware-to-KVM migration. for issue #13007
Changes made:
UnmanagedVMsManagerImpl.javaupdateImportVMTaskErrorState(...)inside theCloudRuntimeExceptioncatch block.ImportVmTasksManagerImpl.javaupdateImportVMTaskErrorState(...)when the provided task object is null.These changes ensure that the original import/conversion failure (for example missing
virt-v2vornbdkit-vddkon the target KVM host) is preserved and propagated correctly to the UI instead of being hidden by a secondary NPE during task error handling.