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

refactor(publisher): Remove PublishError in favor of anyhow::Result #312

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jurshsmith
Copy link
Member

@Jurshsmith Jurshsmith commented Nov 14, 2024

This commit removes PublishError in favor of bubbling pending unactionable errors to stdout when there is an error publishing any subject/payload.

Given the focus of the Publish process, only dependent libraries or actionable errors should have an error signature. We can safely do this since it's all application code and we will be relying on our tracing for stack traces to implement certain actions after analyzing the errors. This makes our code more concise for the domain solution it solves.

Closes DS-108

This commit removes PublishError in favor of bubbling pending
unactionable errors to std when there is an error publishing
any subject/payload.

Given the focus of the Publish process, only dependent libraries or
actionable errors should have an error signature.We can safely do this
since it's all application code and we will be relying on our tracing
for stack traces to implement certain actions after analyzing the errors.
This makes our code more concise for the domain solution it solves.
@Jurshsmith Jurshsmith self-assigned this Nov 14, 2024
@Jurshsmith Jurshsmith changed the title refactor(publisher): remove PublishError in favor of anyhow::Result refactor(publisher): Remove PublishError in favor of anyhow::Result Nov 14, 2024
Copy link
Contributor

@0xterminator 0xterminator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIce. WOnder if we should also add .context("xxx") from the anyhow crate for extra info to some of the errors, or maybe not ?

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