Sitelet https://github.com/nodejs/node/issues/54601
Skip to content

Internal Assertion in AbortSignal #54601

Closed
Closed
Internal Assertion in AbortSignal#54601
Labels
abortcontrollerIssues and PRs related to the AbortController and AbortSignal APIs.confirmed-bugIssues and PRs for confirmed bugs.web-standardsIssues and PRs related to web-platform APIs and standards compliance.

Description

@avivkeller
avivkeller
opened on Aug 28, 2024

I am working on updating the Web Platform Tests in #54468, but ran into an issue occuring with AbortSignal. I narrowed down the issue to a minimal reproduction, with leads to an ERR_INTERNAL_ASSERTION:

const controller = new AbortController();
const signal1 = AbortSignal.any([controller.signal]);

controller.signal.addEventListener('abort', () => {
    AbortSignal.any([signal1]);
})

controller.abort();
node:internal/event_target:1090
  process.nextTick(() => { throw err; });
                           ^

Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

    at assert (node:internal/assert:14:11)
    at AbortSignal.any (node:internal/abort_controller:253:11)
    at AbortSignal.<anonymous> (/repro.js:5:17)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:816:20)
    at AbortSignal.dispatchEvent (node:internal/event_target:751:26)
    at abortSignal (node:internal/abort_controller:374:10)
    at AbortController.abort (node:internal/abort_controller:396:5)
    at Object.<anonymous> (/repro.js:8:12)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10) {
  code: 'ERR_INTERNAL_ASSERTION'
}

Node.js v22.7.0

The failed assertion is

node/lib/internal/abort_controller.js

Line 253 in 885692a

assert(!sourceSignalRef.aborted);

It seems to me that Node.js's current behavior is to fail the assertion when a signal has already aborted, however the WPT's expected behavior is that "Dependent signals for AbortSignal.any() are marked aborted before abort events fire":
https://github.com/web-platform-tests/wpt/blob/e78446e34a1921371658a5df08c71d83f50a2a2f/dom/abort/resources/abort-signal-any-tests.js#L193

Activity

  1. avivkeller
    added
    repro-exists
    abortcontrollerIssues and PRs related to the AbortController and AbortSignal APIs.
    on Aug 28, 2024

  2. avivkeller commented on Aug 29, 2024

    @avivkeller
    avivkeller
    on Aug 29, 2024
    MemberAuthor

    Spec: https://dom.spec.whatwg.org/#create-a-dependent-abort-signal

  3. shubhamsugara22 commented on Aug 30, 2024

    @shubhamsugara22
    shubhamsugara22
    on Aug 30, 2024

    Dependent abort signal that allow one or more 'AbortSignal' object to depend on each other
    the dependent signal will automatically aborted if the parent is aborted . So the links explains the behavior that AbortSignal should behave like that but it fails assertion when signal has already been aborted .
    Looks like you can test the case by skipping the signal if aborted or check if the signal is aborted creating an exception handler
    this 2 case can be tested to see if the impact and early test if we modification can be made to this
    if you have any other way please discuss?
    @redyetidev

  4. avivkeller commented on Aug 30, 2024

    @avivkeller
    avivkeller
    on Aug 30, 2024
    MemberAuthor

    I'm sorry, I'm not exactly following your comment, could you elaborate?

    The issue, to clarify, is that in internal assertion is thrown in a place where a signal should be returned (to be WPT compliant).

  5. avivkeller
    added
    web-standardsIssues and PRs related to web-platform APIs and standards compliance.
    on Aug 30, 2024

  6. shubhamsugara22 commented on Aug 30, 2024

    @shubhamsugara22
    shubhamsugara22
    on Aug 30, 2024

    😞 Sorry for that i meant this:
    What is understood
    A dependent abort signal is designed so that when the parent signal is aborted, the dependent signal should automatically follow suit. The DOM specification outlines this behavior, but it seems that Node.js currently fails this internal assertion when a signal has already been aborted.
    Solution we can try ?
    One possible way to address this would be to check if the signal has been aborted before attempting to create a dependent signal. If it has, we could skip it or handle it with an exception. This would help ensure compliance with the WPT. We could start by testing these approaches to see if they resolve the issue.

    If you have any other suggestions or ideas on how to ensure WPT compliance while avoiding this assertion, I'd love to discuss them!

  7. avivkeller commented on Aug 30, 2024

    @avivkeller
    avivkeller
    on Aug 30, 2024
    MemberAuthor

    Ahh okay. If you have an idea/improvement, opening a PR is a great way to contribute

  8. shubhamsugara22 commented on Aug 31, 2024

    @shubhamsugara22
    shubhamsugara22
    on Aug 31, 2024

    yeah , thanks will try to open one for this

  9. avivkeller
    added
    confirmed-bugIssues and PRs for confirmed bugs.
    and removed
    repro-exists
    on Sep 10, 2024
  10. jasnell
    closed this as completedin d473606on Sep 21, 2024
  11. targos
    added a commit that references this issue on Oct 4, 2024

    lib: propagate aborted state to dependent signals before firing events

    f7c3b03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    abortcontrollerIssues and PRs related to the AbortController and AbortSignal APIs.confirmed-bugIssues and PRs for confirmed bugs.web-standardsIssues and PRs related to web-platform APIs and standards compliance.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions