镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

stream: throw instead of destroy? #33192

Description

@rexagod

I've come across multiple instances (in http2, for example) where due to a condition not being fulfilled, an error was thrown rather than destroying the stream itself (which would emit an error event and hence would be easier to work with).

I wanted to know if there's any reason behind not destroying the stream, or if this needs to be fixed?

Activity

  1. changed the title [-]stream: `throws` instead of `destroy`?[/-] [+]stream: `throw` instead of `destroy`?[/+] on May 1, 2020
  2. ronag commented on May 1, 2020

    @ronag
    Member

    I wanted to know if there's any reason behind not destroying the stream, or if this needs to be fixed?

    This has been discussed a few times in the past. Most recently in #31818.

    Some refs:

    #31831 (comment)
    #31831
    #31818

  3. ronag commented on May 1, 2020

    @ronag
    Member

    It's a case of logic (throw) vs runtime (destroy) errors.

  4. rexagod commented on May 2, 2020

    @rexagod
    MemberAuthor

    Thank you for the quick response, ronag!

    Quoting @mscdex from #31818 (review),

    Not throwing errors goes against behavior found throughout node.

    Throwing should be used when there is a user error of one kind or another that can be immediately detected: when the stream has already ended (something that the user could check easily first), if an argument of the wrong type is passed, etc.

    I'll work on a PR soon to ensure that this logic is implemented in streams (and HTTP[s]\1\2).

  5. ronag commented on May 3, 2020

    @ronag
    Member

    I'll work on a PR soon to ensure that this logic is implemented in streams (and HTTP[s]\1\2).

    Please note that this is not currently the consensus. Throwing is not always preferred. Please read the rest of the discussion.

  6. BridgeAR commented on May 23, 2020

    @BridgeAR
    Member

    I am closing this as there does not seem anything in particular actionable.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions