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

Add setters for the failure and starvation reporters in IOApp #4010

Open
wants to merge 1 commit into
base: series/3.x
Choose a base branch
from

Conversation

BalmungSan
Copy link
Contributor

@BalmungSan BalmungSan commented Feb 19, 2024

As promised in Discord.
c.c. @armanbilge @djspiewak


I will fix tests if we decide to move forward with this approach.
For now, I am more interested in gathering feedback about the proposal.

Comment on lines +256 to +260
// Eventually this will be needed:
// reportFailure = t =>
// IO(_failureReporter)
// .flatMap(_.apply(t))
// .unsafeRunAndForgetWithoutCallback()(runtime)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IORuntime.createEventLoop does not receive a reportFailure argument.
Thus, I guess native either doesn't have this problem or the reporting functionality has not been implemented yet. I am assuming the latter, thus I decide to leave this ready to be called once that functionality is implemented.

@fredfp
Copy link

fredfp commented Mar 19, 2024

As reporter of #3993 I'm wondering if the starvation reporter is anything special here.

Errors are usually reported by printing to sdterr (e.g., cats.effect.IOApp.onNonMainThreadDetected), I was wondering if we should allow overriding the part that logs to the console/stderr (i.e., for redirection to a logger).

It is in that sense that I'm asking if starvation reporter is anything special, would it make sense to rather add (and allow overriding) something like def reportError(error: String): IO[Unit] and call this to report starvation, non main thread detected and other issues?

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.

3 participants