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

Synchronous task completion #104

Open
timdp opened this issue Aug 14, 2018 · 3 comments
Open

Synchronous task completion #104

timdp opened this issue Aug 14, 2018 · 3 comments

Comments

@timdp
Copy link

timdp commented Aug 14, 2018

We've been using Listr in lots of projects here, so first of all: 👏

Unfortunately, I've also run into a little annoyance and I can't seem to find an elegant workaround. I'd basically like to synchronously exit on failed tasks, but still update the renderer first:

const Listr = require('listr')
const Observable = require('zen-observable')

const tasks = [
  {
    title: 'Die after 1s',
    task: () => new Observable(observer => {
      setTimeout(() => {
        observer.error(new Error('Dead'))
        process.exit(1)
      }, 1000)
    })
  }
]

new Listr(tasks).run()

In this code, I would expect the error state of the observable to immediately update the renderer. However, because the observable actually gets translated to a promise internally, all error handling gets scheduled. (The same presumably applies to task completion.) The result is that the process exits with the task stuck on the last state of the spinner.

I know Listr has the exitOnError option (which I should be setting to false above), but I actually don't want to use it. In the actual code, Listr is just one of many progress reporters, while error reporting gets delegated to a generic handler.

I tried forcibly calling render() but that doesn't do anything since the task promise hasn't actually been rejected yet. Delaying the process.exit() call solves it, but the whole point of my code is to exit synchronously in order to avoid side effects. Additionally, Task#run() contains quite a bit of logic so there's no way for me to short-circuit the error handling logic, and that'd be pretty messy anyway.

I think this could theoretically be solved by refactoring Listr to be built on top of observables rather than promises, as that would allow for synchronous updates if necessary. While that sounds like a reasonably big undertaking, I wouldn't mind taking it on as a weekend project. However, I wanted to get your thoughts first.

Thanks!

@okonet
Copy link
Contributor

okonet commented Sep 8, 2018

We kind'a have similar problems in 🚫💩 lint-staged where we doing some sort of workarounds to postpone errors printing until Listr finishes: lint-staged/lint-staged#421

@timdp do you think it would simplify this as well?

@timdp
Copy link
Author

timdp commented Sep 8, 2018

@okonet That PR seems to deal with printing the right error rather than making sure that it gets printed altogether, right? I'll take a look at the rest of the code though, you've tickled my interest.

Big fan of lint-staged btw. 👍

@okonet
Copy link
Contributor

okonet commented Sep 8, 2018

Yes, it prevents is from printing asap and postpones it until completion

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

No branches or pull requests

2 participants