[JENKINS-19584] Ability to redirect user to configuration screen when erroneous data is saved

[timja]

When a user enters erroneous data in the configuration screen and hits Save, we should be able to redirect the user back to the page and display a user-friendly error message.

This becomes relevant if the user pastes data into a field and hits Save or simply ignores the UI error message. See JENKINS-19582 for an example.

---

Originally reported by cowwoc, imported from: Ability to redirect user to configuration screen when erroneous data is saved

• status: Open
• priority: Major
• resolution: Unresolved
• imported: 2022/01/10

[timja]

jglick:

Not sure about API design. A special IOException subclass that could be caught by top-level doConfigSubmit and used to redisplay the form page? But then your edits would be lost, or at least all the edits following the error in terms of order of application.

Graying out Save (and Apply) while there are outstanding errors might sound reasonable, but I am afraid that there may be too many situations where a false positive error is displayed—e.g. configuring a native Maven job will complain if you have no Maven installations, yet you might have already added one in another tab, and there is no way to dismiss the message. Perhaps all such cases should be treated as bugs in the code rendering the form validation, i.e. should downgrade to warning?

[timja]

cowwoc:

Jesse,

I don't think it's a problem if APPLY/SAVE is grayed out because the current tab isn't aware of another tab's edits. That's somewhat expected and (in my opinion) better than the current situation.

In general we would be better off if Jenkins was more "RESTful" in handling these cases. Meaning, if the underlying data was changed before you hit APPLY/SAVE, Jenkins should offer some sort of mechanism for merging and/or force you to reload the page. The alternative (clobbering concurrent updates) seems worse.

[timja]

jglick:

The example of unconfigured Maven installations was just one example. I am simply not sure what other cases there may be of false errors from form validation, and it seems risky to prevent users from saving a form when they know that a particular error message does not really represent a problem.

[timja]

rsandell:

The git plugin is one such example. If the master isn't configured correctly for your git environment, but your slaves are (because it is not intended to clone to master or some other securinty measures) then the git plugin will complain that it can't find the repo, but it will clone just fine when it starts to build on a slave.
That is probably an issue with the git plugin, but it is an example of a "false" error.

[timja]

danielbeck:

One other commonly false positive form validation error are the FilePath.checkFileMask related errors in e.g. Archive Artifacts, which often (no workspace) make no sense at all.

Maybe don't disable Save/Apply, but require an explicit confirmation like "There are outstanding validation errors. (If possible enumerate the failing options.) Try to save anyway?".

[timja]

jglick:

If there is no workspace, validateFileMask reports ok() as you would expect. The error status should be reserved for cases where the validation method is reasonably sure something is actually wrong.

However any logic driven off FormValidation would really have to look for warnings, too, since for example a blank required field is normally shown as a warning rather than an error under the expectation that the user is about to fill it in and just has not gotten there yet and showing a red mark on a freshly loaded form is impolite. Not sure if we can defer validation checks until after the field has gotten focus, or if we could introduce a status which is logically like ERROR yet is displayed in a more toned-down UI that merely indicates that a field is required, not that you did something wrong.

I like the idea of showing a dialog when Save (or Apply) is clicked, which shows a list of outstanding warnings and errors.

[timja]

danielbeck:

jglick:

The error status should be reserved for cases where the validation method is reasonably sure something is actually wrong.

Right, wrote that comment without double-checking the conditions. But it still happens a lot for me: builds in progress (updating publishers while the build is already in progress works nicely ), selectively cleaned up workspaces using Workspace Cleanup, and jobs that already ran but are getting reconfigured to create new files. I guess we'll need to see how bad a change like this would be in practice.

[timja]

jglick:

True, even with a best effort there can be false positives from things like checkFileMask. So a dialog listing possible problems would be a good compromise. You could use your own judgement whether the warning/error was meaningful or not. Obviously the code handling the form submission still needs to be written defensively, but it could feel free to just throw an exception if it cannot proceed meaningfully, for example if it expects to store something in a URL-valued field but a malformed URL was entered—whereas without a feature like this there would be pressure to quietly ignore the problem when the form was saved.

[timja]

rsandell:

According to the javadoc of FormValidation, hudson.model.Failure and hudson.model.Descriptor.FormException; throwing any of those should display a user friendly error page, perhaps tweaking those to be able to go back to the correct form state could at least approve things.
Although when I try to throw a hudson.model.Failure I get the standard oops page instead of a "nicer" error page.

[timja]

jglick:

I wonder if it would work to send a 204 response to prevent the browser from rendering any page from configSubmit, so that the form with pending modifications remains open and available for amendment. (Combined with some other mechanism for displaying a popup warning to the user that the submission was rejected.) Not perfect—some form field changes might have actually been applied before Jenkins got to the erroneous one—but better than the current behavior.