I am not too happy this makes it hard to determine an error is cancellation

I am not too happy this makes it hard to determine an error is cancellation

I don't understand. This sets the reason to a TimeoutError whose stack clearly shows it coming from an AbortSignal. And the code still has to be written to use the reason, which is optional. Can you explain?

I don't understand. This sets the reason to a TimeoutError whose stack clearly shows it coming from an AbortSignal. And the code still has to be written to use the reason, which is optional. Can you explain?

Yes, if I have code that might cancel because of a timeout or any other cancellation before .reason I could just check err.name === 'AbortError' which was the consensus reached last time this was discussed.

This method adds a new way to timeout an action that rejects with an error that isn't AbortError. I want to make sure users have a clean way to handle this.

Similar to the AbortSignal.reason, I don't think this actually breaks any patterns. Yes, it sets the reason to the TimeoutError but it's still up to applications to pay attention to that property. Any existing code that handles the abort event that ends up creating and throwing the AbortError instead will continue to work as expected.

CI: https://ci.nodejs.org/job/node-test-pull-request/41110/

@targos ... please take another look. I updated the implementation to ensure that the underlying timer would not prevent the AbortSignal from being garbage collected and to unref the timer object so that the timer would not keep the event loop from exiting.

I'll have a look tomorrow.

CI: https://ci.nodejs.org/job/node-test-pull-request/41182/