Repository navigation
ui: handle empty network offerings without Request failed (#14377) - #14399
nabassa1323 wants to merge 1 commit into
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
| this.networkOfferings = json.listnetworkofferingsresponse.networkoffering || [] | ||
| this.networkOfferings = this.networkOfferings.filter(offering => offering.fornsx === this.selectedZone.isnsxenabled) | ||
| if (!this.selectedZone.routedmodeenabled) { | ||
| const offerings = json?.listnetworkofferingsresponse?.networkoffering || [] |
There was a problem hiding this comment.
looks like #13503 already fixed the crash on the isolated tab for 4.22 and main. does it still show request failed on main without this change?
There was a problem hiding this comment.
Thanks @Damans227 for the review — I was not aware of #13503 when I opened #14377 and this PR.
Our report is on CloudStack 4.22.1.0. #13503 is targeted at the 4.22.2 milestone, so it was not something we had on our installed 4.22.1.0 packages when we hit the bug.
Isolated repro on 4.22.1.0
- Add Network → Isolated (all Isolated network offerings disabled)
- Toast: Request failed. (no HTTP status)
- Network tab: listNetworkOfferings → HTTP 200
- Response:
{ "listnetworkofferingsresponse": {} } - Console:
TypeError: can't access property "filter", t.networkOfferings is undefined
at fetchNetworkOfferingData (CreateIsolatedNetworkForm.vue:555)
Root cause
Same as #13485 / #13503: empty listnetworkofferingsresponse -> networkOfferings stays undefined -> TypeError on .filter -> UI shows "Request failed."
The difference is only the trigger: in our case it is disabling Isolated offerings, not physical-network tags.
Beyond the same || [] guard as #13503, this PR also:
- shows a clear warning on Isolated when no offerings are available (instead of only failing silently / with a toast)
- hardens L2 the same way (|| [] / safe parse) — UI: Prevent failure when loading network offerings when physical network has a tag #13503 only touched Isolated
- adds a catch on listVPCs so a VPC lookup failure does not break the offerings path
- documents that the empty-response crash is also triggered by disabled offerings, not only physical-network tags
I’m happy to slim this PR to the L2 / warning / listVPCs parts, or close whatever is redundant
There was a problem hiding this comment.
thanks, that explains it. can we slim this to the isolated warning and the l2 change? the empty list crash itself is already fixed on 4.22 and main
| this.networkOfferings = [] | ||
| getAPI('listNetworkOfferings', params).then(json => { | ||
| this.networkOfferings = json.listnetworkofferingsresponse.networkoffering || [] | ||
| const offerings = json?.listnetworkofferingsresponse?.networkoffering || [] |
There was a problem hiding this comment.
the shared tab already handles an empty list here in 4.22.1.0. which request fails on that tab in the browser network tab?
There was a problem hiding this comment.
None. On Shared, listNetworkOfferings also returns HTTP 200 with an empty
listnetworkofferingsresponse. No request fails in the Network tab; the
"Request failed" toast comes from the client-side TypeError path, same as Isolated
There was a problem hiding this comment.
in 4.22.1.0 that line already falls back to an empty list and shows the shared warning. can you share the console error from the shared tab? it might be the isolated tab erroring when the dialog opens
|
can we point this at the 4.22 branch? its a fix for 4.22.1.0 so it should go into 4.22.2 and merge forward to main |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14399 +/- ##
============================================
- Coverage 20.21% 20.21% -0.01%
Complexity 20749 20749
============================================
Files 6426 6426
Lines 580181 580195 +14
Branches 71033 71039 +6
============================================
- Hits 117292 117288 -4
- Misses 450174 450195 +21
+ Partials 12715 12712 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
39d4570 to
8f64372
Compare
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@nabassa1323 thanks for slimming it down. the branch is on 4.22 now but the PR still targets main, so it picks up the java 21 packaging commit and shows conflicts. can you switch the base to 4.22 with the edit button next to the title? |
|
@Damans227 Done. PR base switched to 4.22. |
Damans227
left a comment
There was a problem hiding this comment.
thanks @nabassa1323, the isolated warning and l2 fix look right. LGTM
Description
Fixes the Add Network UI showing "Request failed" when Isolated/Shared
have no enabled network offerings (empty/null listNetworkOfferings response).
Shows a clear warning instead. Does not hide guest-type tabs (separate
enhancement from #14377).
Fixes: #14377 (UI error handling part only)
Types of changes
How to test