The intention is to keep to repo root clean while the list of packages
is slowly growing.
Additionally, a bunch of small (~30 LoC) files in the repo root is
merged into a single maddy.go file, for the same reason.
Most of the internal code is moved into the internal/ directory. Go
toolchain will make it impossible to import these packages from external
applications.
Some packages are renamed and moved into the pkg/ directory in the root.
According to https://github.com/golang-standards/project-layout this is
the de-facto standard to place "library code that's ok to use by
external applications" in.
To clearly define the purpose of top-level directories, README.md files
are added to each.
Rules to be generally followed are outlined in HACKING.md (section
Error handling) and in exterrors package documentation.
exterrors.SMTPError is extended with type-safe fields commonly added
using 'arbitrary context' WithFields function.
Most of "reason" field uses are err.Error(), so do that by default
using the Err field.
-- Problem
The way module references are implemented was a big source of confusion,
mainly because in most cases there are at least two possible ways they
are handled. Additionally, for some modules (checks and modifiers) there
is the third way, what doesn't help either.
Consider the following cases of confiugraiton directives:
```
deliver_to smtp_downstream
```
This directive refers to the existing configuration block named
smtp_downstream. It doesn't have to be the instance of the
smtp_downstream module, though.
```
deliver_to smtp_downstream tcp://127.0.0.1:1125
```
This magically turns "reference to an existing block" into an inline
definition. And suddenly the first argument is not an configuration
block name, now it is a module name. Same "sudden" change happens when
the block is added:
```
deliver_to smtp_downstream { ... }
```
For modules having an "implicitly defined" config block, there's
another source of confusion:
```
deliver_to smtp_downstream
```
This directive may refere to the implicitly defined config block with
some default values. But to the user it looks like a module name and
nothing more. It's trickly to explain all dark corners of such behavior
in the user documentation.
Even more, there's another problem with the implementation, these
implicitly defined blocks can't access global directives because they
are defined before the configuration is parsed.
-- Solution
This commit removes the third way of module reference handling. There
are no "implicitly defined" config blocks anymore.
Second, all module references by default create a new module instance.
All following directives create a new module instance, no catches here.
```
deliver_to smtp_downstream
deliver_to smtp_downstream tcp://127.0.0.1:2525
deliver_to smtp_downstream { ... }
```
Although, the first one will fail because smtp_downstream needs at least
one endpoint address somewhere.
Ability to define configuration blocks at a top-level and reference them
in other places is retained for use in cases where it's actually
useful, including the initial idea of "state sharing" (see "Dev:
Comments on design" on the project wiki).
However, such referneces are now explicitly prefixed by the '&'
character. Such as the following:
```
deliver_to &smtp_downstream
```
This directive references the existing configuration block named
"smtp_downstream". Following directives are not allowed as they make no
sense:
```
deliver_to &smtp_downstream tcp://127.0.0.1:2525
deliver_to &smtp_downstream { ... }
```
So, there is no confusion about what happens when.
Closes#167. I decided to not make any radical changes now. Changes
made to the initialization logic solve the actual problem that led to
the creation of the referenced issue.
Previous error reporting code was inconsistent in terms of what is
logged, when and by whom. This caused several problems such as: logs
missing important error context, duplicated error messages, too verbose
messages, etc.
Instead of logging all generated errors, module should log
only errors it 'handles' somehow and does not simply pass it to the
caller. This removes duplication, however, also it removes context
information. To fix this, exterrors package was extended to provide
utilities for error wrapping. These utilities provide ability to
'attach' arbitrary key-value ('fields') pairs to any error object while
preserving the original value (using to Go 1.13 error handling
primitives).
In additional to solving problems described above this commit makes logs
machine-readable, creating the possibility for automated analysis.
Three new functions were added to the Logger object, providing
loosely-typed structured logging. However, they do not completely
replace plain logging and are used only where they are useful (to allow
automated analysis of message processing logs).
So, basically, instead of being logged god knows where and when,
all context information is attached to the error object and then it is
passed up until it is handled somewhere, at this point it is logged
together with all context information and then discarded.
To support unusual configuration syntax, endpoint modules (imap, smtp,
etc) relied on rather awkward code using modName+instName+aliases as
arguments. This commit replaces old handling with use of special
signature similar to inlineArgs introduced in 1edd031.
Endpoint modules are placed in a separate 'registry' and use
different initialization callback signature for simplicity. This makes
them inaccessible for other modules, though they are not supposed to be
anyway.
Endpoint modules are initialized before other modules. This allows
detecting unused configuration blocks by checking for modules
that were not lazily initalized after endpoint initialization.
This relies on endpoint modules being essentially "roots" of
instances dependency tree.
Idea of "semantical module names" is completely dropped now and so
HACKING.md is updated to not mention it.
This commit majorly updates all Markdown documents by attempting to maintain a consistent (80 columns per line) formatting and addresses some display bugs in other Markdown renderers due to improper use of Markdown - GitHub is a little broken. But it also addresses grammar and even makes semantical changes to some paragraphs.
Notably, in README.md, section "System authentication helper binaries" had relatively severe changes.
To future contributors: if trying to maintain any kind of wrapping standard, I suggest using special extensions, like Rewrap for VSCode.
>Shortly put, it is module responsibility to log errors it generated since it is
>assumed it can provide all useful details about possible causes.
Modules can be chained in arbitrary ways so it is not possible to know
whether module should log the error or not. Logging errors at every
stage will result in useless duplication of messages.
Additionally, error source can provide all useful information about the
error context. Functionality for attaching arbitrary context info to the
error objects is not implemented now and its arguable whether it will be
useful.
