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

Fix uncaught exception reporting for Scheduler #888

Merged
merged 6 commits into from Jun 12, 2019

Conversation

Projects
None yet
3 participants
@oleg-py
Copy link
Collaborator

commented May 24, 2019

Fixes #799, #829.

@Avasil

Avasil approved these changes May 24, 2019

Copy link
Collaborator

left a comment

Just a few nitpicks but looks great overall

AsyncScheduler(context, executionModel)

def apply(
ec: ExecutionContext,
reporter: UncaughtExceptionReporter): Scheduler =

This comment has been minimized.

Copy link
@Avasil

Avasil May 24, 2019

Collaborator

Just a nitpick but it has different formatting than method above. :D

This comment has been minimized.

Copy link
@oleg-py

oleg-py May 29, 2019

Author Collaborator

Thanks for nits, actually, it's very helpful 👍

@@ -32,9 +32,19 @@ private[execution] class SchedulerCompanionImpl extends SchedulerCompanion {
*/
def apply(
context: ExecutionContext = StandardContext,
executionModel: ExecModel = ExecModel.Default): Scheduler =
executionModel: ExecModel = ExecModel.Default,

This comment has been minimized.

Copy link
@Avasil

Avasil May 24, 2019

Collaborator

IIRC the trailing comma won't work on all Scala versions

/** An `AsyncScheduler` schedules tasks to be executed asynchronously,
* either now or in the future, by means of Javascript's `setTimeout`.
*/
final class AsyncScheduler private (
context: ExecutionContext,
override val executionModel: ExecModel)
override val executionModel: ExecModel,
reporter: UncaughtExceptionReporter

This comment has been minimized.

Copy link
@Avasil

Avasil May 24, 2019

Collaborator

not sure if it matters at all but on JVM it is in different order:

final class AsyncScheduler private (
  scheduler: ScheduledExecutorService,
  ec: ExecutionContext,
  r: UncaughtExceptionReporter,
  val executionModel: ExecModel)


/**
* A `Runnable` which can be wrapped to

This comment has been minimized.

Copy link
@Avasil

Avasil May 24, 2019

Collaborator

not complete sentence

def run(): Unit = throw Dummy
}

private[this] val immediateEC = new ExecutionContext {

This comment has been minimized.

Copy link
@Avasil

Avasil May 24, 2019

Collaborator

I think we could use monix.execution.schedulers.TrampolineExecutionContext.immediate

oleg-py and others added some commits May 31, 2019

@alexandru

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

Hey guys, this is looking good, so what's left to do on it?

@Avasil

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

@alexandru Oh, I meant to assign for review, to double check but if it's fine then we can merge it :)

@Avasil Avasil merged commit 75a3b16 into monix:master Jun 12, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.