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

Eliminate PublishingError in favor of simple strings, mirroring warnings and ensuring fatal errors crash at the call site. #458

Closed
wants to merge 1 commit into from

Conversation

JPToroDev
Copy link
Collaborator

An improvement on our working error-handling system until we officially tackle the subject.

…rnings` and ensuring fatal errors crash at the call site.
@wti
Copy link

wti commented Feb 10, 2025

I don't understand the motivation, but noticed some possible interactions with content symlinks:

  1. Content symlink (Support symbolic links in Content directory parsing  #264) supports configurable number of errors to tolerate, avoiding fatal errors. The goal is to enable the user to tolerate some content/parsing errors instead breaking the whole site build, on the assumption that it's better to omit the invalid content.
  2. I noticed a "duplicate directory" error and wondered if content-symlink code should be throwing that instead of the one it does.

Having some common enum error types for related errors (e.g., parsing vs assembly vs resource-discovery) with a number of cases would simplify error-handling (perhaps the user could handle and ignore cases at will) and help with discoverability (as here with my finding the existing error). As an interim measure if it's necessary to enforce immediate fatal error in some cases, you could use a registery for enums or swap struct for enum and use an initializer.

The approach of consolidating error creation and handling seems robust to the frequent addition of new errors. But my sympathies in any case; error handling is pervasive and extensive, and it's hard to simplify :)

@JPToroDev
Copy link
Collaborator Author

This PR was meant to make the current behavior better without fully reworking error and warning handling (which is needed and will be done down the line), but there are much more pressing PRs, so I'll close this for the time being.

@JPToroDev JPToroDev closed this Feb 10, 2025
@JPToroDev JPToroDev reopened this Feb 11, 2025
@JPToroDev JPToroDev closed this Feb 11, 2025
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.

2 participants