Functions engineGet and enginePost in fetch.js do not have any error handling in case the request fails. They automatically expect that the response could be parsed to JSON but if the server returns, e.g. 404, it fails with the following message in the browser console. DataProvider failed to fetch data SyntaxError: "JSON.parse: unexpected end of data at line 1 column 1 of the JSON data" The code should be changed to check the response status and if it is not 200 then log it into the console, for example: Response { type: "basic", url: "https://localhost:8443/ovirt-engine/api/hostsx", redirected: false, status: 404, ok: false, statusText: "Not Found", headers: Headers, body: ReadableStream, bodyUsed: true } It would be also good to let clients of the functions know that the request has failed so that they can decide how to deal with it.
Lucia, I agree that a better error handling for http errors is a nice to have here but not sure regarding the priority of this. The json response can be checked if it's valid & not null and if the data is fetched as required and log an error in case it's not (e.g. VmMigrateDataProvider.js). Can I assign this bug to you for fixing it if you still think it's important?
Yes, assign it to me please.
(In reply to Lucia Jelinkova from comment #2) > Yes, assign it to me please. Done, thanks Lucia.
Lucia, do you plan on fixing this for 4.4.4 or should I postpone to 4.4.5?
Lets postpone it to 4.4.5.
Lucia, do you plan on fixing this for 4.4.5? Thanks.
No. But I do have a task in ui-extensions for 4.4.6 so there might be some time to look into this task too in 4.4.6.
(In reply to Lucia Jelinkova from comment #7) > No. But I do have a task in ui-extensions for 4.4.6 so there might be some > time to look into this task too in 4.4.6. So postponing to 4.4.6. Thanks.
(In reply to Sharon Gratch from comment #8) > (In reply to Lucia Jelinkova from comment #7) > > No. But I do have a task in ui-extensions for 4.4.6 so there might be some > > time to look into this task too in 4.4.6. > > So postponing to 4.4.6. Thanks. Hi Lucia, is this still feasible for 4.4.6?
Sorry, I am on a leave so it won't make 4.4.6. Moving to 4.4.7.
Lucia, same(In reply to Sharon Gratch from comment #9) > (In reply to Sharon Gratch from comment #8) > > (In reply to Lucia Jelinkova from comment #7) > > > No. But I do have a task in ui-extensions for 4.4.6 so there might be some > > > time to look into this task too in 4.4.6. > > > > So postponing to 4.4.6. Thanks. > > Hi Lucia, is this still feasible for 4.4.6? The same question, just for 4.4.7:)
I have some work to do in ui-extensions in 4.4.7 and I'll have a look at this issue too.
Since entering the FF and the patch on ui-extensions side is still under review, let's postpone it for 4.4.8 (hopefully last postponing for this).
Lucia, something seems wrong with current fix. Before this fix, in case of an error for example on the "export VM" dialog, the following original self explained rest api error appears: attachment 1801865 [details] While now, a non user friendly error appears instead: attachment 1801866 [details] And this is the error as appears in dev tools console log: attachment 1801867 [details] const doExportVm = async () => { try { const result = await onExportVm(vm.id, exportVmName, selectedStorageDomain, shouldCollapseSnapshots) console.log('export result:', result) close() } catch (e) { console.error('export problem:', e) setError(e.message) } } Seems like the e.message doesn't include the original rest api error which is not good.
The errors sent from the engine REST API have different format. The ui-extensions parsed the case when error contained the details message directly, but the case when the error contained the fault object which contained detailed message was missing. The attached patch fixes that (and displays only the detailed message, as it was before).
(In reply to Lucia Jelinkova from comment #18) > The errors sent from the engine REST API have different format. The > ui-extensions parsed the case when error contained the details message > directly, but the case when the error contained the fault object which > contained detailed message was missing. The attached patch fixes that (and > displays only the detailed message, as it was before). Looks much better now, thanks for the info. Only thing still disturbing is this error prefix text of "Danger alert" as appears after latest fix, e.g. attachment 1816952 [details] it's not so user friendly and definitely not a danger situation so I think we need to remove that text.
I have not changed the dialog, just the message. Maybe this came with some PF update - when I inspect the page I can see the following: <span class="pf-u-screen-reader">Danger alert:</span>
(In reply to Lucia Jelinkova from comment #21) > I have not changed the dialog, just the message. Maybe this came with some > PF update - when I inspect the page I can see the following: > > <span class="pf-u-screen-reader">Danger alert:</span> Lucia, correct, this is generated by: https://github.com/oVirt/ovirt-engine-ui-extensions/blob/6b7687aec43c3e624cbc30371ca43988aa80b318/src/modals/vm-export/VmExportModalBody.js#L66 and it seems that it came with this patch changes: https://gerrit.ovirt.org/115819 Scott, can you please take a look?(In reply to Lucia Jelinkova from comment #21) > I have not changed the dialog, just the message. Maybe this came with some > PF update - when I inspect the page I can see the following: > > <span class="pf-u-screen-reader">Danger alert:</span>
There is a strange gap between the react version we have in ui-extensions with patternfly CSS that is rendered out. I'm experimenting locally to see what will work to have the "Danger alert:" text hidden again. PF4 adds the span to better support screen reader tech.
Hello Lucia, please what dialogs have been affected by this? I confirmed that the export VM dialog does show the error to the user; however, migrate dialog does not show the error if the VM is not running for example. Could you please provide verification steps? Thank you
Hi Ivana, the issue was mainly about the error handling in the code when calling engine api and logging the errors into the console log of the browser and in the ui.log on the server. Unfortunately, every dialog handles the error messages shown to the user differently - some show it within the dialog, some as a toast in webadmin and some do not show it at all (like migration dialog). But that is not part of this issue. Feel free to open a new BZ for the migration dialog not showing error. For verifying this BZ I would suggest: 1. Verify the changed export dialog 2. For some of the other dialogs, check if the console of the browser and ui.log file on the server contain the error
This bugzilla is included in oVirt 4.5.0 release, published on April 20th 2022. Since the problem described in this bug report should be resolved in oVirt 4.5.0 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report.