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

Incorrect short-circuiting of "task runner"? #384

Open
tdd opened this issue Jan 25, 2017 · 1 comment
Open

Incorrect short-circuiting of "task runner"? #384

tdd opened this issue Jan 25, 2017 · 1 comment

Comments

@tdd
Copy link

tdd commented Jan 25, 2017

Hi Nicholas,

Location: Chapter 11 > Page 238 > "Promise-Based Asynchronous Task Running" > code at top of page

The following code block almost at the top puzzles me:

if (err) {
  result = task.throw(err);
  return;
}

You're side-stepping (pun intended) the call to the next step(), which doesn't seem right. In fact, your next take on it does take care to step() after the task.throw(err).

What am I missing here?

@rsmeral
Copy link
Contributor

rsmeral commented Apr 1, 2018

I wouldn't call it incorrect per se. It it inconsistent though, using return after error in the callback version, and using step() after error in the Promise version.

How I understand what happens in case of error, in both versions:

  1. Runner gets the generator and obtains iterator
  2. Runner gets the callback/promise object by running the first task.next()
  3. The generator runs and yields once
  4. Runner calls the async function (result.value)
  5. The async function fails (the runner either sees err on callback, or ends up in catch clause of the Promise)
  6. And here's the problem – the task.throw calls the generator, which, in this example, doesn't handle the error thrown at it, and thus the error bubbles up the stack. Given that the error isn't caught anywhere in the generator or the runner, program execution halts, and execution never gets to the following return/step() in the runner, so it doesn't really matter whether step() is there or not. Even the return is unnecessary.

However, if the generator handled the error, then it would make sense to have either return or step(). I see this as a choice when designing the runner. You can have either:

  • with return – a runner which stops executing code of the task after the task encounters and handles an error – strange behaviour, I can't imagine a use case (not saying one does not exist)
  • with step() – a runner which continues execution of the task if the task can handle its errors – which is behaviour I would expect

The decision to abort (handling or not handling the error and returning or not returning) should be up to the task, not up to the runner.

So, I agree that the use case with return is at least strange, if not incorrect :)
(Or I'm just completely misunderstanding something ¯\_(ツ)_/¯ )

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