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 monitor function which doesn't block parent from shutting down #58

Closed
wants to merge 2 commits into from

Conversation

jnicklas
Copy link
Collaborator

@jnicklas jnicklas commented Jan 6, 2020

This comes up time and again, where we want to have a fork, but do not want the fork to block the parent from shutting down. Where this is common is in the error monitor pattern that has emerged in bigtest-server. Hence the function name.

So for example:

  let errorMonitor = fork(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });

This is tricky because the error monitor will block the fork from shutting down. This is likely not what we want. Instead we want the monitor to shut down if the parent exits. To accomplish this today, we need to wrap the entire rest of the function in a try-finally.

  let errorMonitor = fork(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });

  try {
    ...
    yield
    ...
  } finally {
    errorMonitor.halt();
  }

This is both ugly and error prone. With the monitor function we can write this more naturally:

  monitor(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });

@jnicklas
Copy link
Collaborator Author

jnicklas commented Jan 6, 2020

@cowboyd I'm running into this so much now, and since we've already decided that we need this, and the implementation is easy, I thought it best to just fix it.

This comes up time and again, where we want to have a fork, but do not want the fork to block the parent from shutting down. Where this is common is in the error monitor pattern that has emerged in bigtest-server. Hence the function name.

So for example:

```
  let errorMonitor = fork(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });
```

This is tricky because the error monitor will block the fork from shutting down. This is likely not what we want. Instead we want the monitor to shut down if the parent exits. To accomplish this today, we need to wrap the entire rest of the function in a try-finally.

```
  let errorMonitor = fork(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });

  try {
    ...
    yield
    ...
  } finally {
    errorMonitor.halt();
  }
```

This is both ugly and error prone. With the monitor function we can write this more naturally:

```
  monitor(function*() {
    let [error]: [Error] = yield on(child, "error");
    throw error;
  });
```
@frontsidejack
Copy link
Member

A preview package of this pull request has been released to NPM with the tag monitor.
You can try it out by running the following command:

$ npm install effection@monitor

or by updating your package.json to:

{
  "effection": "monitor"
}

Once the branch associated with this tag is deleted (usually once the PR is merged or closed), it will no longer be available. However, it currently references effection@0.4.0-6a8b6aa which will be available to install forever.

Generated by 🚫 dangerJS against 6c77671

@jnicklas jnicklas mentioned this pull request Jan 6, 2020
This is useful for building re-usable abstractions with monitor
@frontsidejack
Copy link
Member

A preview package of this pull request has been released to NPM with the tag monitor.
You can try it out by running the following command:

$ npm install effection@monitor

or by updating your package.json to:

{
  "effection": "monitor"
}

Once the branch associated with this tag is deleted (usually once the PR is merged or closed), it will no longer be available. However, it currently references effection@0.4.0-324bd7d which will be available to install forever.

Generated by 🚫 dangerJS against fdf388c

@cowboyd
Copy link
Member

cowboyd commented Jan 15, 2020

This is definite win. Like fork, I think it can be implemented as an operation using #57

@cowboyd
Copy link
Member

cowboyd commented Jan 29, 2020

superseded by #62

@cowboyd cowboyd closed this Jan 29, 2020
@cowboyd cowboyd deleted the monitor branch March 30, 2021 13:22
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.

None yet

3 participants