Developers of Sympa

[IKEDA Soji <address@concealed>]

On Mon, 16 Jun 2014 16:45:32 +0200
Guillaume Rousse <address@concealed> wrote:

<<snip>>

> This make sense, and should indeed allow more consistency between 
> messages. I just have two remarks.
> 
> First, I don't think we need so much categories (8), when some of them 
> are uneeded and some are quite fuzzy, and we'd rather merge some of them 
> for sake of simplicity.
> 
> - debug, debug2, and debug3: do we really need 3 different levels ? 
> without any definition of their respective intents ?

I agree to that there are too many levels for debugging.  These
would be reduced --- I rather think it is not neccessary.

IMO currently there are two sorts of usage.

- Usage for tracing and profiling.

  Another way to output more precise information might be used,
  e.g. L<Debug::LTrace>.

- Usage like "printf debug".

  When fulfilled suit of unit tests are prepared, such lines need
  not be kept in submitted code.

  Those prefer to this usage may use "trace" level and remove it
  before submission (n.b. our Subversion server rejects submission
  including "trace" lines).

I suppose "debug" level may be kept but there may not be multiple
levels.

> - info and notice: what's an 'epochial event' ? Why should we make a 
> distinction between 'some kind of success' and 'other kind of success' ?

"notice" means what the administrators are interested in.
"info" means others: Facts like who mistyped password, who is added
to which list, how many a list distributes messages in a day,
... and so on are relatively less important for administrators.

Conversely, "info" and "err" levels mean what users are interested
in.  So IMO these levels should be in sync with these:
- Action log - generated by db_log(), web_db_log().
- Statistics - generated by db_stat_log(), web_db_stat_log().
- Reports - sent/responded by functions in L<report>.
- Notifications - sent by send_notify_to_*().
- DSN - sent by send_dsn().
In current code, synchronization is incomplete.  Call to do_log()
should always appear along with functions above --- they may rather
call do_log() internally.

How about this idea?

> - alert and crit: we don't need two different levels just for enforcing 
> two different reactions. Especially when you classify 'program 
> unexpectedly dies' under 'alert' category, whereas you defined 'crit' as 
> 'Unexpected situations' :)

I don't disagree to merge "alert" and "crit" levels.

However, note that "alert", "crit" and "err" levels correspond to
Sympa itself, foreign component and users' action, respectively.

On "alert" level, Sympa has no choice but to die.  That's why I
added the case of "unexcepted" death to example: other cases will
cause expected death.

OTOH by "crit" and "err" levels, she (or he?) should _not_ die.

In short, I don't categorize levels by which is "expected" or
"unexpected".


> Basically, I'd rather go for the four categories:
> - fatal error
> - error
> - information
> - debug
> 
> Mapping those levels into syslog priorities is another exercice.
> 
> Second, there is a confusion between 'how to classify an error' vs 'how 
> to react to an error'. Basically, we don't need two different categories 
> for unrecoverable errors, whereas the context is enough to provide 
> different responses.
> 
> Expected unrecoverable errors should be handled by specific error 
> handlers, so as to deliver human-readable and context-specific error 
> messages. For example, we should make a distinction between 'database 
> unavailable' and 'database available, outdated schema' errors. And I 
> don't see the need of a stack trace here, if we're able to pinpoint the 
> actual issue.
> 
> Unexpected unrecoverable errors should be handled by the default error 
> handler (the one installed as a signal handler), acting as a safety net, 
> but its purpose should be only to deal with uncaught exceptions in other 
> parts of the code, not to replace all kind of error handling. Here it 
> make sense to output a call trace.
> 
> That's more or less the current situation now, with 
> terminate_on_expected_error() (previously Sympa::Log::fatal_err()) and 
> terminate_on_unexpected_error() (previously Sympa::Site signal handler), 
> in Sympa::Tools::Daemon, excepted I didn't change anything in their 
> original behaviour.
> 
> Also, as long as the configuration is not loaded, the logging subsystem 
> is quite limited to STDERR.

I suppose fatal_err() is not neccessary.

It may be used only on top-level.  Because it does not give
backtrace.  At present, simply die()'ing is enough.

Regards,

--- Soji

-- 
株式会社 コンバージョン  セキュリティ&OSSソリューション部   池田荘児
〒231-0004　神奈川県横浜市中区元浜町3-21-2 ヘリオス関内ビル7F
e-mail address@concealed  TEL 045-640-3550
http://www.conversion.co.jp/