Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chk_cyrus: Fix fatal error on nonexistent mailboxes #4805

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

caldwell
Copy link

I have these mailboxes on my system:

user.me.Junk (\HasChildren)
user.me.Junk.Learn (\NonExistent \HasChildren)
user.me.Junk.Learn.Ham (\HasNoChildren)
user.me.Junk.Learn.Spam (\HasNoChildren)

As far as I understand it's perfectly valid to have a nonexistent parent in imap.

However, when I run chk_cyrus it gets a fatal error on user.me.Junk.Learn:

$ chk_cyrus -M user.me.Junk.Learn
Examining mailbox: user.me.Junk.Learn
bad mailbox user.me.Junk.Learn in chkmbox
fatal error: fatal error

I investigated and found that the fatal error was IMAP_MAILBOX_NONEXISTENT. It seems to me that shouldn't be a fatal error in chk_cyrus.

This PR has 2 small commits:

  1. One of them prints the error out. With just this commit the output of chk_cyrus becomes:
    $ chk_cyrus -M user.me.Junk.Learn
    Examining mailbox: user.me.Junk.Learn
    bad mailbox user.me.Junk.Learn in chkmbox: Mailbox does not exist
    fatal error: fatal error
    
  2. One them changes chk_cyrus to just skip over nonexistent mailboxes:
    $  chk_cyrus -M user.me.Junk.Learn
    Examining mailbox: user.me.Junk.Learn
    $
    

Does this seem reasonable?

@elliefm
Copy link
Contributor

elliefm commented Mar 25, 2024

What version of Cyrus are you using? As of a few versions ago (I'm not sure which, perhaps 3.4 or 3.6), Cyrus no longer produces these nonexistent intermediate mailboxes, intermediate path tokens are now always created as full mailboxes. I don't remember why exactly. That might be why chk_cyrus is complaining about them.

On servers that have been around for a while and upgraded over the years, they can of course still exist, but in that case you should probably create them as real mailboxes. So perhaps chk_cyrus should report them as missing, but not fatal out.

The change to make the fatal error log the error message looks good. The other change, to ignore the nonexistent error code, looks fine for what it is, but I'm not sure what behaviour we actually want here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants