You are here: Foswiki>Tasks Web>Item9403 (01 Aug 2010, GeorgeClark)Edit Attach

Item9403: Configure doesn't detect or report back errors found in the Sanity checks.

pencil
Priority: Urgent
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Component: configure
Branches:
Reported By: GeorgeClark
Waiting For:
Last Change By: GeorgeClark
BasicSanity.pm performs a series of checks against LocalSite.cfg. However it fails to increment the errors counter - $this->{errors}++; for most of the detected errors, including syntax and compile errors in the file. the ->insane() function returns insane if errors were detected. So configure proceeds as if LocalSite.cfg was missing rather than reporting that the actual file is there but damaged. it also doesn't report back errors such as being unable to create or write to LSC.

However there is also a missing piece. If errors are reported, bin/configure reports them as a simple print to the cgi, resulting in an unformatted error screen for the user, with no way to proceed from there.

  • Configure needs to show the errors in the context of the configure panels
  • BasicSanity needs to increment the counter for detected errors.

As this could result in a user loosing their existing configuration after a compile error in LSC, making urgent for 1.1

-- GeorgeClark - 28 Jul 2010

I've made a fix that will pass back the BasicSanity errors into the configure interface, and done some basic testing with corrupted configs. Reporting seems fine, but save will append good config onto bad config. FoswikiCfg::save doesn't know about the insane state of the config file

The advantage is that a fixable config won't get overwritten, but a bad config cannot be fixed by configure if the admin doesn't have shell access, like on some hosting sites. So I think the real fix should also propagate the insane state into the Configure UI, so save starts a new file instead of appending to a corrupted file.

However I'm a bit lost in the configure class structure and am not quite sure how to proceed. I will check in the reporting fix, so that the admin is informed of the configuration problem

-- GeorgeClark - 29 Jul 2010

That's a tough call. If an admin with access to LSC suddenly finds they are forced to start with a new file, it could be a touch frustrating. On the other hand, propagating a broken LSC is unhealthy.

What might be most effective is kicking into a textarea/edit mode on a broken LSC. That way a hosted person can still rescue it (if they know what they are doing, or/and can get help).

-- CrawfordCurrie - 29 Jul 2010

If LSC is corrupted, they visit configure and get an INTERNAL ERROR warning, including a note that if they save they might loose their old configuraion. At that point nothing has been changed. If they then save the file, it currently appends, leaving the file corrupt, but now with duplicated items. If the save would overwrite, the admin still has an opportunity to offline edit, but if they cannot then save would at least restore a usable config

-- GeorgeClark - 29 Jul 2010

I've change it so now the admin is told that the file is corrupt, and that if they proceed with save, the old settings will be lost. They should manually update the file to correct the problem.

Setting to closed - not an issue with 1.0.x

As Crawford suggests, a textarea / editor for the LCS would be the better solution. Maybe something like the "about:config" editor on firefox. Sounds like a good feature for 1.2

-- GeorgeClark - 01 Aug 2010
 

ItemTemplate edit

Summary Configure doesn't detect or report back errors found in the Sanity checks.
ReportedBy GeorgeClark
Codebase trunk
SVN Range
AppliesTo Engine
Component configure
Priority Urgent
CurrentState Closed
WaitingFor
Checkins distro:08d252e87580 distro:5357acf9dd91 distro:d7a8f09be766
TargetRelease minor
ReleasedIn 1.1.0
Topic revision: r7 - 01 Aug 2010, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy