Created attachment 1240029 [details] console.log, engine.log, server.log Description of problem: I got an Uncaught exception in UI when tried to create NFS domain with string in 'Retransmissions (#)' field. Version-Release number of selected component (if applicable): ovirt-engine-4.1.0-0.4.master.20170111194730.git0a1100c.el7.centos.noarch How reproducible: Always Steps to Reproduce: 1. Create NFS domain with string in 'Retransmissions (#)' text box Actual results: UI.log: 2017-01-12 18:51:05,583+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-2) [] Permutation name: 0661FF244C5EA5656FC338D0C4E7609A 2017-01-12 18:51:05,584+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-2) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at Unknown.at bz(Unknown Source) at Unknown.at Object.xv(Unknown Source) at Unknown.at Object.Fv(Unknown Source) at Unknown.at Object._8(Unknown Source) at Unknown.at Object.d9(Unknown Source) at Unknown.at l8(Unknown Source) at Unknown.at Vp(Unknown Source) at Unknown.at Object.dq(Unknown Source) at Unknown.at V3(Unknown Source) at Unknown.at Xp(Unknown Source) Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at Unknown.anonymous(Unknown Source) at Unknown.at Object.g3(Unknown Source) at Unknown.at oQ(Unknown Source) at Unknown.at Object.q3(Unknown Source) at Unknown.at Object.sKm [as Zf](Unknown Source) at Unknown.at Object.Q5(Unknown Source) at Unknown.at Object.P3(Unknown Source) at Unknown.at v8(Unknown Source) at Unknown.at l8(Unknown Source) at Unknown.at Vp(Unknown Source) at Unknown.at Object.dq(Unknown Source) Expected results: No exception Additional info: console.log, engine.log, server.log
Elad - can this be reproduced with debuginfo? Otherwise, the stack trace is obfuscated. It has to match the relevant webadmin package version - and you'll get it in ui.log.
Created attachment 1249784 [details] engine.log, server.log, ui.log (2) Modified "<!-- Loggers for the UI -->" in /usr/share/ovirt-engine/services/ovirt-engine/ovirt-engine.xml.in to DEBUG. I get the same exception in ui.log: 2017-02-13 11:28:15,637+02 INFO [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] GWT symbolmaps are not installed, please install them to de-obfuscate the UI stack traces 2017-02-13 11:28:15,648+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] Permutation name: F13FE4CACE40D24F999C20570C1DE9D3 2017-02-13 11:28:15,648+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at Unknown.Av(webadmin-0.js@29429) at Unknown.Iv(webadmin-0.js@41) at Unknown.l9(webadmin-0.js@19) at Unknown.p9(webadmin-0.js@19) at Unknown.x8(webadmin-0.js@117) at Unknown.Wp(webadmin-0.js@26) at Unknown.eq(webadmin-0.js@24224) at Unknown.f4(webadmin-0.js@149) at Unknown.Yp(webadmin-0.js@112) at Unknown.vAf(webadmin-0.js@28145) at Unknown.f4e(webadmin-0.js@85) at Unknown.h6e(webadmin-0.js@46) at Unknown.Qx(webadmin-0.js@29) at Unknown.Ux(webadmin-0.js@57) at Unknown.eval(webadmin-0.js@54) Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at Unknown.s3(webadmin-0.js@28) at Unknown.IQ(webadmin-0.js@28) at Unknown.C3(webadmin-0.js@198) at Unknown.cJm(webadmin-0.js@10468) at Unknown.a6(webadmin-0.js@2463) at Unknown._3(webadmin-0.js@858) at Unknown.H8(webadmin-0.js@209) at Unknown.x8(webadmin-0.js@59) ... 10 more rhevm-4.1.0.4-0.1.el7.noarch ovirt-engine-webadmin-portal-4.1.0.4-0.1.el7.noarch
Elad - are you sure the debuginfo version matches the webadmin package? (and restart engine after you've verified they are the same)
Created attachment 1249874 [details] engine.log in DEBUG (3) Installed ovirt-engine-webadmin-portal-debuginfo, reproduced again: 2017-02-13 16:34:09,612+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-13) [] Permutation name: F13FE4CACE40D24F999C20570C1DE9D3 2017-02-13 16:34:09,614+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-13) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at java.lang.Throwable.fillInStackTrace(Throwable.java:114) [rt.jar:1.8.0_121] at java.lang.RuntimeException.RuntimeException(RuntimeException.java:32) [rt.jar:1.8.0_121] at com.google.web.bindery.event.shared.UmbrellaException.UmbrellaException(UmbrellaException.java:70) [gwt-servlet.jar:] at com.google.gwt.event.shared.UmbrellaException.UmbrellaException(UmbrellaException.java:25) [gwt-servlet.jar:] at com.google.gwt.event.shared.HandlerManager.$fireEvent(HandlerManager.java:117) [gwt-servlet.jar:] at com.google.gwt.user.client.ui.Widget.$fireEvent(Widget.java:127) [gwt-servlet.jar:] at com.google.gwt.user.client.ui.Widget.fireEvent(Widget.java:127) [gwt-servlet.jar:] at com.google.gwt.event.dom.client.DomEvent.fireNativeEvent(DomEvent.java:110) [gwt-servlet.jar:] at com.google.gwt.user.client.ui.Widget.$onBrowserEvent(Widget.java:163) [gwt-servlet.jar:] at com.google.gwt.user.client.ui.ValueBoxBase.onBrowserEvent(ValueBoxBase.java:240) [gwt-servlet.jar:] at com.google.gwt.user.client.DOM.dispatchEvent(DOM.java:1648) [gwt-servlet.jar:] at com.google.gwt.user.client.impl.DOMImplStandard.dispatchEvent(DOMImplStandard.java:320) [gwt-servlet.jar:] at com.google.gwt.core.client.impl.Impl.apply(Impl.java:296) [gwt-servlet.jar:] at com.google.gwt.core.client.impl.Impl.entry0(Impl.java:335) [gwt-servlet.jar:] at Unknown.eval(webadmin-0.js@54) Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) __gwt$exception: <skipped>: Cannot read property 'cc' of undefined at com.google.gwt.editor.client.impl.SimpleError.SimpleError(SimpleError.java:33) [gwt-servlet.jar:] at com.google.gwt.editor.client.impl.AbstractEditorDelegate.$recordError(AbstractEditorDelegate.java:133) [gwt-servlet.jar:] at com.google.gwt.editor.ui.client.adapters.ValueBoxEditor.getValue(ValueBoxEditor.java:82) [gwt-servlet.jar:] at org.ovirt.engine.ui.common.editor.UiCommonEditorVisitor$2.onKeyPress(UiCommonEditorVisitor.java:86) at com.google.gwt.event.dom.client.KeyPressEvent.dispatch(KeyPressEvent.java:78) [gwt-servlet.jar:] at com.google.gwt.event.shared.GwtEvent.dispatch(GwtEvent.java:76) [gwt-servlet.jar:] at com.google.web.bindery.event.shared.SimpleEventBus.$doFire(SimpleEventBus.java:173) [gwt-servlet.jar:] ... 11 more rhevm-4.1.0.4-0.1.el7.noarch ovirt-engine-webadmin-portal-debuginfo-4.1.0.4-0.1.el7.noarch
This problem seems to be existing in many places in the webadmin, an input field is defined an ShortEntityModelTextBoxEditor/IntegerEntityModelTextBoxEditor and when a non number value is entered in the input a validation failure appears in the dialog yet the entity within the model is being set to null as the editor tries to parse the value to a short/int and does not succeed. Since the model gets null in that field, the validation succeeds and the save action is executed and the wrong value in the field is being completely ignored. Vojtech, Greg, what do you suggest to do in that case? I've checked and it also happens in the VM popup when trying to set illegal values in the RNG device
Created attachment 1280787 [details] nfs dialog master
(In reply to Tal Nisan from comment #5) > This problem seems to be existing in many places in the webadmin, an input > field is defined an > ShortEntityModelTextBoxEditor/IntegerEntityModelTextBoxEditor and when a non > number value is entered in the input a validation failure appears in the > dialog yet the entity within the model is being set to null as the editor > tries to parse the value to a short/int and does not succeed. > Since the model gets null in that field, the validation succeeds and the > save action is executed and the wrong value in the field is being completely > ignored. > Vojtech, Greg, what do you suggest to do in that case? I've checked and it > also happens in the VM popup when trying to set illegal values in the RNG > device It's a obviously a bug that the parse failure allows a form to submit. IMO that's what should be fixed. I'm not sure of where in the framework that change goes. @Vojtech or Alexander?
[btw, I duplicated the validation failure bug on master, but I didn't get a JavaScript error. See screenshot.]
Well the proper fix is to fix the model validator to not allow null values for the integer field. Tal hit the problem right on the head. The problem is that the integer parser fails to parse the value (since its not an integer) and a null is passed to the model as a result (I tried really hard to modify this behavior but couldn't). A while ago I implemented some code to properly give an error in the UI in this particular case, however if the model validator itself allows the null to be properly validated, then the model is at fault. Simply fixing the MODEL validator to not allow null will solve the issue. Btw the model should have never allowed null to begin with if it is not a valid value to pass to the backend regardless of if the UI gives an error.
When sync'ing the model (in this case, NfsStorageModel) from UI editor widgets, UiCommonEditorVisitor#setInModel is called. Check out the GWT-generated class NfsStorageView_retransmissions_entity_Context.java located under webadmin/gen directory. This is the class behind `ctx` param in UiCommonEditorVisitor#setInModel method when dealing with `Retransmissions (#)` input field. The actual model-bound GWT widget is EntityModelTextBox (extends GWT ValueBox). The GWT editor (ShortEntityModelTextBoxEditor) sets it up with custom renderer & parser. The general contract is that the parser (e.g. ToShortEntityModelParser) returns null on empty input and throws exception on invalid input. > when a non number value is entered in the input a validation failure appears in the dialog yet the entity within the model is being set to null as the editor tries to parse the value to a short/int and does not succeed. Check out EntityModelTextBox#getValue override. In GWT editor framework, the general HasValue#getValue contract is that both empty and invalid value is treated as `null`. When sync'ing the model from UI editor widgets, we should adapt our models accordingly, as they are the ones handling the validation. > Since the model gets null in that field, the validation succeeds and the save action is executed and the wrong value in the field is being completely ignored. What happens is the following call // see NfsStorageView_retransmissions_entity_Context parent.getRetransmissions().setEntity(null) due to invalid UI input. As I wrote above, both empty input and invalid input are represented by the same `null` value. > Well the proper fix is to fix the model validator to not allow null values for the integer field. Empty input is valid input for UI fields whose values are optional. That's the main reason (I think) why the model validator contract (IValidation) allows nulls. Our problem is the lack of differentiation between empty vs. invalid input, both treated as `null`, which makes our validators think the input is empty and therefore passes the validation logic. One solution is to augment HasEditorValidityState interface with method like: willStateBeValid(T value) and modify UiCommonEditorVisitor#setInModel to be like: if (editorIsValidNow && editorWillBeValid) { // proceed to call ctx.setInModel } that is the infra-level solution. Another solution is to do fix this on model-level, however I'd prefer an infra-level fix to address all the models (scope == any editor implementing HasEditorValidityState).
I've discussed this with Alex, basically UI infra (editor framework) currently sets the EntityModel's value to null if the user-entered input is either empty or invalid. Most validators (including IntegerValidation) pass on null value. This is by design, as null generally means "missing value". Perhaps NfsStorageModel#validate should use NotNullIntegerValidation instead? getRetransmissions().validateEntity(new IValidation[] { new NotNullIntegerValidation(0, RETRANS_MAX) }); Tal, what do you think?
> This problem seems to be existing in many places in the webadmin Can we identify those places? We should fix all of them. > and the save action is executed and the wrong value in the field is being completely ignored. Forgot to mention this - the "wrong value" translates to null which passes validation. This should be OK from infra perspective.. in such cases, model's validation should reject nulls, e.g. using NotNullIntegerValidation.
I forgot to add as I thought it was obvious, I was referring to optional values that the user can enter i.e. null is allowed and if a value is entered it's supposed to be validated so in that case not null validation will not work
Okay now I am confused. From the MODELs perspective what is the difference between the value being null from the user not entering a value and the value being null from the user entering an invalid value? It appears you are saying that in the second case somehow we have an exception when loading the model from the backend after its been saved while we don't in the first case? I don't understand how either case is any different from the model perspective.
Let's take for instance the retransmissions number when creating a new NFS domain, you can leave it as null which is completely OK, the value will just not be used when sending the mount command or you can enter a custom value which should be a valid number. What happens de facto is if you enter a value of "300," by mistake when you meant to enter 300, instead of the dialog notifying you that you entered an invalid value, the new connection will be saved without a custom number of retransmissions
If you entere 3oo instead of 300 the field will be marked as an error and if the dialog allows a submission, then that is a bug in the dialog the infra structure will have marked the field in error. I know this works as for instance in the fence agent edit dialog you do this it will not allow you to save. So I don't know what you want us to do? The infra will indicate the field is in error, the dialog validation ignores it and grabs the value (which will be null since it can't parse the value), and that happens to be a valid value for the field.
Verified in ovirt-engine-4.1.4.1-0.1.el7.noarch