Content error handling

[wti]

In the context of #265, we noticed that any content parsing error could break the build, and considered some way for site builders to tolerate content errors since they come from other authors, particularly with symlinks. This is to work out that design before doing a PR.

All designs have a policy declared as a property on Site, with a default implementation that can be overridden to specify the alternative. During content processing, that property modulates error handling. Previously, any error was thrown, but tolerated errors are printed instead.

Initially it was just to tolerate some error count:

maxContentErrors: Int

Then in https://github.com/twostraws/Ignite/blob/main/Tests/IgniteTesting/Publishing/ContentErrorToleranceTests.swift

/// Whether to tolerate a content error.
public enum ContentErrorTolerance: CustomStringConvertible {
    /// No errors are tolerated
    case NONE

    /// Permit when name has the specified suffix (case-sensitive)
    case filename(endsWith: String)

    /// Permit when the deploy path has the specified prefix (case-sensitive)
    case deployPath(startsWith: String)

   /// Permit when the message contains some String
  case message(contains: String)

    /// Permit up to (and including) a certain number
   case upTo(max: Int)

    /// Permit when any of these are matched
    indirect case anyOf([ContentErrorTolerance])

    /// Permit any except for these (i.e., fail if any match)
    indirect case anyExcept([ContentErrorTolerance])

    public func tolerate(
      count: Int,
      url: URL,
      deployPath: String,
      error: any Error
    ) -> Bool {
      ...
    }
}

Then the name was changed to ContentErrorPolicy and anyOf and anyExcept changed to [ContentErrorPolicy] in #265 (comment)

As an aside in terms of process, I would ask that in discussions, we avoid editing comments or only add edits to the bottom of a comment. E.g., this comment (#265 (comment)) was edited 8 times to change the objection and what was proposed in light of later discussions. I find it disconcerting because I end up wondering why I didn't address the proposal or where the original comment was, when the latest version was written after later discussion and code.

[wti]

(Aiming for one issue per comment thread)

re: proposal to change name to ContentErrorPolicy in #265 (comment)

The name ContentErrorTolerance reflect that the only semantics supported are to permit some errors, and the API and cases both reflect that: {tolerate}.upTo(max: 10) or {tolerate}.filename(endsWith: "tmp.md"). A more general error policy might support levels (info/warning/error/fatal) or some handlers, etc.

The more general name is viable because the API uses the term tolerate(...), so one could imagine other policy aspects implemented with other API's. I just haven't seen or imagined those other aspects yet.

The more general name might be preferred if it's to be included in an aggregate error policy (to distinguish it from SiteErrorPolicy and ...?), but that would need to be fleshed out first.

[wti]

Regarding the proposal to remove anyOf and anyExcept in favor of [ContentErrorPolicy] in #265 (comment)

I'm not sure what the composite semantics are supposed to be for an array: ignore an error if it matches any? All? Is there a way to use the negative, of ignoring all errors except these?

That's what anyOf and anyExcept make clear. With anyOf, an error is tolerated if it matches any predicate, and with anyExcept an error is tolerated unless it matches the predicate.

There are still gaps in what you can say. E.g., you can't say e.g., permit filenames ending with draft.md but only up to 10 such errors. However, aiming for a theoretically complete solution could be overkill pending user interest, unless it's supposed to be a model for error policies in other contexts.

[wti]

As an objection to the current proposal, using case-sensitive matching for the filename suffix does not track the policy of case-insensitively matching the content suffix for purposes of selecting content.

It's less clear the deploy path prefix should be case-insensitive. Would it help when a case-insensitive Site is reading from a case-sensitive volume reached via symlink?

[wti]

When matching error messages, the current proposal renders error to string for matching using string interpolation. Is that the correct approach? (Using .localizedDescription caused a crash.)

[JPToroDev]

Let's start with the high-level stuff first:

1. Happy to retain anyOf and anyExcept and have the final Site property return a single ContentErrorTolerance instead of an array of them. Thank you for elaborating.

I'll leave it to you to recommend a name for the final enum—any names I've thrown out are merely suggestions. 🙌🏼

[wti]

[Edit: changed "error" here to "mistake"]

The current proposal composite anyOf and anyExcept permit users to make illogical combinations. Should we support an API to report those [mistakes]?

Not all [mistakes] are discoverable, but we could at least report when the children [of anyOf or anyExcept] are empty or themselves are composites.

Similarly, it's possible to specify an empty string for the simple predicate matches, or to specify something like a glob (filename(endsWith: "*.md"). We could detect and report these.

Assuming some mistake is reportable, it seems to be a Site configuration error that warrants throwing an error to fail the Site build. Is that correct?

[JPToroDev]

There's a lot going on in that last comment—in quite a few of these comments. 😅

And I'm having a bit of trouble parsing everything.

So let's focus on one problem at a time.

The current proposal composite anyOf and anyExcept permit users to make illogical combinations. Should we support an API to report those errors?

What's your proposed solution?

[wti]

There's a lot going on in that last comment

Sorry, the last comment (#532 (comment)) is about a feature to address invalid enum specifications. I think that's one topic (i.e., not a lot), and it's comprised of the aspects I mentioned. Feel free to add missing aspects, and address/correct the ones presented.

There's a lot going on ... in quite a few of these comments. 😅

Would you identify which comments are too complicated and how? I can best respond when feedback is specific.

[JPToroDev]

I'd like to focus on your second to last comment and break it down first.

Could you clarify how the user's ability to make illogical combinations of content error policies relates to error discoverability?

[wti]

(start-edit: adding context, assuming my response without context was confusing)

The current proposal composite anyOf and anyExcept permit users to make illogical combinations. Should we support an API to report those errors?

What's your proposed solution?

Reply follows: a method to describe mistakes in forming a content error enum:

(end-edit)

public enum ContentErrorTolerance {
  ...
  /// Describes why enum is ill-formed, or nil if well-formed
  public var invalidInput: String? { ... }
}

final class PublishingContext {
  init(
        for site: any Site,
        ...
    ) throws -> PublishingContext {
    ...
    if let err = site.contentErrorTolerance.invalidInput {
        throw SomeError.contentSpecificationErr(err)
    }
  }
}
...

[wti]

This discussion seems stalled, but I believe there are only 3 questions remaining.

(1) This more complex enum unavoidably supports meaningless input, which should be a (non-maskable) Site error. The proposal is to throw an error when the user specifies invalid input, and enable the user to discover the error without running a build by fetching the invalidInput field on the enum.

(2) When tolerating errors by catching and printing them, how should we render them to string? The proposal is string interpolation, but I feel that's likely wrong given the complications of LocalizedError, NSError, etc. I would expect some library function to recover text via more specific typing, but I haven't found one.

(3) For tolerating errors by deploy-path prefix or filename suffix, the proposal is to use case-sensitive matching for deploy-path but case-insensitive for filename, in order to match the approach taken when selecting content.

I'm inclined to go ahead with these proposals, but happy to do otherwise.