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

tty: fix TypeError when stream is closed #43803

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 12, 2022

Alternative to #41335 that got stalled.
Fixes: #41330

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tty Issues and PRs related to the tty subsystem. labels Jul 12, 2022
@aduh95 aduh95 added the review wanted PRs that need reviews. label Aug 1, 2022
@aduh95
Copy link
Contributor Author

aduh95 commented Aug 1, 2022

/cc @nodejs/tty

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it useful to handle this gracefully? I would rather see an error that the stream is already destroyed? Otherwise it's difficult to know what happens as a user.

@aduh95
Copy link
Contributor Author

aduh95 commented Aug 16, 2022

@BridgeAR see the linked issue, and I have also been annoyed by that error in other occasions. I struggled to find a good way to reproduce the error reliably, and I agree that the test as-is doesn't feel like something we would necessarily want to support, but the issue is real, and it looks like to me that fixing it is less problematic than not fixing it.

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2022
@nodejs-github-bot nodejs-github-bot merged commit ab89024 into nodejs:main Aug 24, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in ab89024

@aduh95 aduh95 deleted the fix-tty-TypeError branch August 25, 2022 14:06
sidwebworks pushed a commit to sidwebworks/node that referenced this pull request Aug 26, 2022
RafaelGSS pushed a commit that referenced this pull request Sep 5, 2022
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
targos pushed a commit that referenced this pull request Sep 16, 2022
juanarbol pushed a commit that referenced this pull request Oct 10, 2022
@juanarbol juanarbol mentioned this pull request Oct 11, 2022
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. review wanted PRs that need reviews. tty Issues and PRs related to the tty subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node in terminal window gives error - Cannot read property 'setRawMode' of null
4 participants