promises and post-mortem debugging

@Raynos

A PR that aims at adding a promises-based API to most node core APIs was recently submitted to nodejs/node.

I have expressed my concerns regarding the impact of such a change on post-mortem debugging, and a separate issue was created by @Raynos to further discuss these concerns.

My concerns are more questions that I asked because I felt that the way promises handle errors is incompatible, at least in some ways, with post-mortem debugging. So far my investigations only confirmed these concerns, but I'm not done with them. One part of these investigations is also to find a solution to the problems that I found so far.

The discussion in nodejs/node has been productive so far, but I represent only one post-mortem debugging user/maintainer. I feel it would help if some people from this working group would share their opinion on these concerns with the rest of the group here in this issue, if only to confirm or infirm that they have the same questions/reserves as I do about the impact of nodejs/node#5020 on post-mortem debugging. For instance, if some of you already worked on that problem and did a similar investigation, I would be interested if you could share your results.

In any case, I will share my findings with the group as soon as I have some more time to continue my investigations.

Thank you!

For what it's worth, I have all the same concerns and reservations, but nothing additional to add to the discussion that you didn't already mention. Thanks for weighing in on the PR.

To what extent are these incompatibilities an implementation detail of V8's Promises vs. an issue with any Promise implementation based on the spec in ES2015?

I ask because we have to approach fixing them very different depending.

@mikeal

@mikeal That's a great question. I would think that we would try to come up with a solution similar to what has been done for --abort-on-uncaught-exception. That is: it would be behind a flag, and should be considered as a debugging tool specific to the Node.js runtime/platform.

I don't think aborting on an uncaught exception when passing --abort-on-uncaught-exception on the command line conforms to the ES 2015 spec, in the sense that I'm not sure if it's in the scope of the spec. If that's a valid interpretation of the spec, I would think the same reasoning could apply to a similar solution to this specific problem.

Also, see nodejs/node#5084 (comment) for a quick experiment that I've done.

@domenic

Maybe we should ask @domenic about this.

What is the exact question?

I suspect it mostly boils down to this: Given the way that promises are currently specified, is it possible for the interpreter to know a priori whether or not a thrown exception will be handled as a rejection later?

If it is possible to know, we can abort the process at the correct moment because we know the thrown exception will not be handled as a rejection at a later time. This preserves the state of the process at the moment the fault occurs; stack arguments will be available, and the heap will have been preserved in its entirety by the operating system.

If it is not possible to know for all cases, then the interpreter must necessarily unwind the stack and continue executing until a subsequent determination can be made as to whether the rejection is unhandled. If this is true, then promises are anathema to post mortem debugging -- the model prescribes the destruction of the very state we are trying so hard to preserve for later study.

I suspect it mostly boils down to this: Given the way that promises are currently specified, is it possible for the interpreter to know a priori whether or not a thrown exception will be handled as a rejection later?

No. Just like there's no way for the "interpreter" to know whether or not a thrown exception will be catch-ed later.

The entire point of promises is to be able to handle errors, similar to try/catch. If I understand you correctly, post-mortem debugging is anathema to the JavaScript language and its try/catch feature (as well as the promises feature that depends builds on that). A better approach would be to run a source transform to replace throw (as well as promise reject calls) with process.abort(), or use a language which does not have catchable exceptions.

No. Just like there's no way for the "interpreter" to know whether or not a thrown exception will be catch-ed later.

This is manifestly false. Whether a throw will be caught by a try/catch or not depends entirely on whether the interpreter is handling code within a (potentially nested) try block or not. The call frame itself can make this coarse grained information available, and the interpreter can walk stack frames without dismantling them.

If nothing else, there is proof by construction: the Node --abort-on-uncaught-exception flag does this today.

The entire point of promises is to be able to handle errors, similar to try/catch. If I understand you correctly, post-mortem debugging is anathema to the JavaScript language and its try/catch feature (as well as the promises feature that depends builds on that).

It's certainly possible to make software less debuggable with blanket usage of try and catch, but you at least have to opt in to doing so. One of the more unfortunate aspects of Javascript is that catch is a binary construct: either you catch everything, or you catch nothing. On this basis, we tend to recommend that you avoid throwing or catching at all. Thrown exceptions should be left to serious programmer errors, where the only recourse is to abort an apparently logically inconsistent program.

Promises, like catch, do not appear to afford the programmer the ability to catch only a subset of expected exceptions. They also apparently lack any way to turn off the indiscriminate swallowing of all classes of exceptions. That is what makes them undesirable, at least to those of us working on systems software.

A better approach would be to run a source transform to replace throw (as well as promise reject calls) with process.abort(), or use a language which does not have catchable exceptions.

Even if reaching into arbitrary software and replacing random statements was a good idea, it would not work for a ReferenceError or a TypeError, or any other fault that arises implicitly due to an errant program. It would be similarly unhelpful with exceptions thrown from functions with a native implementation; e.g., JSON.parse().

This is not about aborting the process every time something is thrown, but rather when something is thrown but not handled.

Given the tone of this conversation, I do not feel comfortable continuing to participate in it, and am unsubscribing from this thread. I would ask that do you do not mention me again in it.

@jclulow

@jclulow: To answer your original question — "Given the way that promises are currently specified, is it possible for the interpreter to know a priori whether or not a thrown exception will be handled as a rejection later?" The answer is no — or at least, not in a way that is compatible with the behavior you're looking for. Rejections are not treated like panics because it is possible that the rejection will become handled later in execution, possibly on a subsequent tick. Note, this is not the same thing as "on error resume" behavior, like domains enable — a rejection cannot "jump out of" the dependency graph of async ops like domains can.

This is not to say promises are anathema to post mortem debugging: as Domenic suggested, relying on throws to panic means that authors need to avoid try and catch where possible. While this group recommends that approach in order to make existing post-mortem tools work optimally, others find value in using try/catch (and promises) for exceptional control flow; and the introduction of constructs like process.on('uncaughtException'), and domains themselves show that the "on error panic" approach is not universally acceptable for Node users — some users and applications will want to make different tradeoffs than systems programmers.

Given that, even absent promises, our existing post-mortem tools rely on specific approaches to writing JS, and given that it's unlikely that generalizing a single approach to writing JS will work for all Node users, it seems to me that finding a solution that addresses the majority of both audience's concerns would be a good outcome to work towards.

For example: the post-mortem working group currently requires panic behavior to happen at the apex of the stack, which means that any approach to writing JS that involves catch clauses reduces the efficacy of the post mortem tools we have available. This happens even if the error is rethrown from within the handler immediately. @misterdjules has noted that the reason for this invariant is the need to reduce the number of confounding factors when examining the state of the program at crash — to be able to rely on the fact that the content of the heap and stack sections did not change between entering the error state and the process exiting.

However, it seems like the most valuable ephemeral information is the stack information — the functions, receivers, and arguments that get destroyed by unwinding the stack. This is also the most difficult requirement imposed by our post-mortem tools — it means that Node users that choose to use catch or promises are opting out of post-mortem techniques, and also requires that constructs like domains are called in a specific manner that can be awkward to ensure.

The Error.captureStackTrace API is backed by a stack frame iterator that has access to all of the information discarded by stack unwinding, and it captures data at precisely the point we need it to. Right now it doesn't record all of the information we're interested in — namely, parameter values. I understand that it doesn't enforce the ideal assertion — that no operations happened between unexpected state and abort — but it brings us from "no information" on catch and rethrow (or promises) to "useful partial information," and allows the post-mortem WG to relax the requirement that panic must happen at top of stack somewhat.

With regards to promises, because the unhandledRejection hook is exposed at application level, users who wish to treat unhandled rejections as panics are free to do so, and may abort providing a useful error object on-stack for post-mortem debugging.

This is not to say promises are anathema to post mortem debugging: as Domenic suggested, relying on throws to panic means that authors need to avoid try and catch where possible.

This point has been repeated a few times in a few threads, so it's worth clarifying. It seems to imply that developers that care about postmortem debugging have to take special care in their code (and, by logical extension, the modules that they use). But the vast majority of operational errors in Node and Node modules are already asynchronous and so are not thrown and not caught with try/catch anyway. As a result, most of the time, things work regardless of whether a module author considered postmortem support in the way they designed an interface or whether a program author considered that when they added a new module dependency.

Unlike try/catch, promises are used for handling asynchronous operational errors. That's why a promises-based core API is significantly more problematic for postmortem debugging than try/catch is.

This is also the most difficult requirement imposed by our post-mortem tools
...
For example: the post-mortem working group currently requires panic behavior to happen at the apex of the stack, which means that any approach to writing JS that involves catch clauses reduces the efficacy of the post mortem tools we have available.

I think it's fairer to say that these requirements derive from the unfortunate behavior of software that sometimes fails in ways that requires that data to understand it. For the reasons I mentioned above, I think in practice the way "catch" is used is not a major problem.

@davepacheco

@davepacheco:

This point has been repeated a few times in a few threads, so it's worth clarifying. It seems to imply that developers that care about postmortem debugging have to take special care in their code (and, by logical extension, the modules that they use). But the vast majority of operational errors in Node and Node modules are already asynchronous and so are not thrown and not caught with try/catch anyway. As a result, most of the time, things work regardless of whether a module author considered postmortem support in the way they designed an interface or whether a program author considered that when they added a new module dependency.

Thanks for your response. This is a fair point: at the moment most errors generated by the core API are propagated through means other than throwing. However, I don't believe this precludes users from having to take special care in order to make post-mortem debugging viable for their programs.

When I say that special care has to be taken, I am referring to the recommendations on the Joyent error document as well as the recent recommendations from the Node error symposium, which stipulate that users should avoid using catch wherever possible in order to avoid losing stack information. In the case of promises, their existing use in the ecosystem would seem to preclude the application of post-mortem tools to those packages (& users of those packages.) Whether Core provides a promise-based API or not, this is a problem with the current state of post-mortem WG, if indeed we wish all Node users to be post-mortem WG users, as suggested elsewhere.

While I understand that core providing a promise-based API highlights this problem, I don't think it introduces it, and indeed it gives us an opportunity to find workable solutions for users that prefer to use catch + rethrow as well as users that prefer promises. Note that this isn't a suggestion to change the practices involved with callbacks — they remain as they are, and the existing tools and techniques used to debug them still work. This is a purely additive suggestion — that we start to look into instrumenting error objects more at stack trace capture time — that makes all programs, callback-based or not, produce more detailed information in core dumps. In other words, this is an opportunity to make the post-mortem debugging story a little better for all Node users, not just the ones that are able (or willing) to make the tradeoffs inherent in the recommendations we've given them thus far.

Related issue about error handling mcollina/split2#8

I suspect it mostly boils down to this: Given the way that promises are currently specified, is it possible for the interpreter to know a priori whether or not a thrown exception will be handled as a rejection later?

The answer is yes, it's possible and v8 is capable of doing it, at least in debug mode.

@vkurchatkin

@vkurchatkin

Thank you very much for taking the time to respond to my questions and taking a look at the latest potential solution I explored. This is exactly the kind of feedback I was looking for, and I'm hopeful that we're starting to be on the right path to find a way that would be acceptable for post-mortem debugging users :)

Isolate::Throw should just be concerned by whether there's a try/catch handler in the stack above the current stack frame.

The problem is that there is ALWAYS try/catch handler in the stack above in case of promises

Absolutely, I think that's well understood, and that's the reason why in my suggested approach I explored a way to not setup a try/catch handler when passing a flag that aims at enabling a more post-mortem debugging compatible behavior.

so we have to special case it

Right, the difference between our potential solutions is where this special casing happens.

This sample code should not abort when run with --abort-on-uncaught-exception.

This is debatable. This patch doesn't allow to throw out of promise handlers and executor regardless of outer try catches. The semantics is weird, indeed, but try-catch can't affect this: promise constructor doesn't throw anything ever.

Would that work as designed with async/await? I'm even less familiar with async/await than with promises, so I may very well be wrong, but from what I understand one could write code such as:

async function boom() {
  await promiseThatThrows();
}

try {
  boom();
} catch () {
}

Is that correct? If so, I would think that even when enabling post-mortem debugging specific behaviors that abort early, that code would not abort.

your approach is interesting, but it clearly violates standard.

Would you mind referring to the section in the standard that indicates that? I'm not familiar with it and it can sometimes be difficult for me to understand e.g what is a hard requirement and what is undefined behavior.

Also I think not catching errors from microtasks can have unforeseen consequences.

Right, thank you for bringing that up. Are microtasks used for anything else than promises and Object.observe? Are there some npm modules using microtasks outside of these two use cases?

@misterdjules

@misterdjules

async functions are basically just functions that return promises, although it is implicitly wrapped in try-catch as well, so it can only reject, not throw.

Would you mind referring to the section in the standard that indicates that?

Section 25.4.3.1 specifies how promise constructor works. As you can see, it always returns normal completion (step 11) expect for 10b, which should't happen. Normal completion means that function does't throw (throwing is an abrupt completion. If new Promise can't throw, wrapping it in try-catch can't catch anything.

Are microtasks used for anything else than promises and Object.observe

no, as far as I know

Are there some npm modules using microtasks outside of these two use cases

there are no good reasons to use them, they are no better than nextTick, but require native code compilation, so most likely the answer is no

@misterdjules

@misterdjules What about generators and transpilers for async/await that compile to generators? The generator based runner must attach a catch handler to the yielded promise, even if there is no try-catch block. Unless it can ask the generator iterator something like generatorIterator.isFrameWithinTryBlock(). Or am I missing something again?

@vkurchatkin

@vkurchatkin

async functions are basically just functions that return promises, although it is implicitly wrapped in try-catch as well, so it can only reject, not throw.

What about this sample code (same as the previous one, without the use of the async keyword to define an async function):

try {
  await promiseThatThrows();
} catch () {
}

Wouldn't that need to not abort regardless of the use of a flag to make uncaught errors in promises abort?

Or maybe it's designed/speced to be rewritten as a promiseThatThrows().catch() by all implementations and thus your approach would work as designed even in this case?

@spion

@spion

What about generators and transpilers for async/await that compile to generators? The generator based runner must attach a catch handler to the yielded promise, even if there is no try-catch block. Unless it can ask the generator iterator something like generatorIterator.isFrameWithinTryBlock(). Or am I missing something again?

Can you be more specific? I don't understand in your comment what the question is. There's a lot of parallel discussions in this thread, and thus I'm not sure what you're referring to. Would you mind quoting the text you're responding to and proving details about what code doesn't work as you would expect and why?

Thank you!

@misterdjules

@misterdjules sorry, I forgot to give an example.

With this change, I'm assuming we would expect this to throw:

function makeRejectedPromise() {
  return Promise.resolve().then(_ => { throw new Error("oops") })
}

var asyncFn = async function f() {
  var result = await makeRejectedPromise();
  return result;
}

f()

because the await call is not within a try-catch block.

However, most existing async function compilers transform async functions to generators using a generator wrapper like this one:

function async(gen) {
  return function() {
    var iterator = gen.call(this, arguments)
    function process(val) {
      // simplified version without sync error handling
      var next = iterator.next(val);
      return next.done ? next.value
                       : next.value.catch(e => iterator.throw(e)).then(process)
    }
    return process()
  }
}

In order for the wrapper to be able to forward errors to the generator iterator, it has to catch the exception. Since we don't have generatorIterator.isFrameWithinTryBlock(), we can't ask it before we attach the catch handler.

Which as far as I can tell means that we wont get an abort (the catch handler is there and checks out), not until iterator.throw(e) throws us the error back (because the generator doesn't handle it). By that point, however, the stack changed completely.

Cross-ref: nodejs/node#13437

@refack

@refack Can you expand on why nodejs/node#13437 is relevant to this discussion?

@misterdjules

@misterdjules I believe it's more the other way around, this discussion is relevant to the one going on at nodejs/node#13437

@refack

@refack The question still stands: how is it relevant? Otherwise it's really not clear to me, and I suspect for others, why the two are linked.

In the most basic sense both discussions deal with Promises and diagnostics (although pre vs post mortem).
In a more specific sense async_hooks enable implementing trace mechanisms that may be beneficial for post-mortem diagnostics, but only if done well.
IMHO this discussion has raised good points, and personally helped me when thinking about pre-mortem Promise tracing. And so I believed cross referencing the two might help others.

_{as an aside; I feel that there is a huge body of knowledge in the nodejs org, but it's quite fragmented, causing mistakes to be repeated or decisions to be made without taking into account points that were already riesed but forgotten. So I feel defragmenting, and cross referencing, more often than not has more benefit than cost}

as an aside; I feel that there is a huge body of knowledge in the nodejs org, but it's quite fragmented, causing mistakes to be repeated or decisions to be made without taking into account points that were already riesed but forgotten. So I feel defragmenting, and cross referencing, more often than not has more benefit than cost

Indeed, I had no idea this thread existed :o

Discussion on this topic has moved to nodejs/promises-debugging.

@mmarchini

As @mmarchini says, this work is now being done in https://github.com/nodejs/promises-debugging. If this needs to be tracked outside of that repository, please open an issue over in https://github.com/nodejs/diagnostics.

Navigation Menu

Repository navigation

promises and post-mortem debugging #16

94 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

promises and post-mortem debugging #16

Description

Activity

davepacheco commented on Feb 6, 2016

mikeal commented on Feb 6, 2016

misterdjules commented on Feb 6, 2016

misterdjules commented on Feb 6, 2016

mikeal commented on Feb 6, 2016

domenic commented on Feb 6, 2016

jclulow commented on Feb 6, 2016

domenic commented on Feb 6, 2016

jclulow commented on Feb 6, 2016

domenic commented on Feb 6, 2016

chrisdickinson commented on Feb 7, 2016

davepacheco commented on Feb 7, 2016

chrisdickinson commented on Feb 7, 2016

stevemao commented on Feb 7, 2016

vkurchatkin commented on Feb 7, 2016

94 remaining items

misterdjules commented on Feb 16, 2016

vkurchatkin commented on Feb 16, 2016

spion commented on Feb 16, 2016

misterdjules commented on Feb 16, 2016

misterdjules commented on Feb 16, 2016

spion commented on Feb 17, 2016

refack commented on Jun 4, 2017

misterdjules commented on Jun 5, 2017

refack commented on Jun 5, 2017

misterdjules commented on Jun 5, 2017

refack commented on Jun 5, 2017

AndreasMadsen commented on Jun 5, 2017

mmarchini commented on Aug 15, 2018

richardlau commented on Aug 15, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions