FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tty: fix TypeError when stream is closed by aduh95 · Pull Request #43803 · nodejs/node · GitHub

/ node Public

tty: fix TypeError when stream is closed - #43803

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
aduh95:fix-tty-TypeError
Aug 24, 2022
ConversationCommits (1)ChecksFiles changed
Merged

tty: fix TypeError when stream is closed#43803
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
aduh95:fix-tty-TypeError

Conversation

aduh95 commented Jul 12, 2022

Copy link
Copy Markdown
Contributor

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

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 force-pushed the fix-tty-TypeError branch from 5697cf3 to 9e6812f Compare August 1, 2022 15:20
aduh95 added the review wanted PRs that need reviews. label Aug 1, 2022

aduh95 commented Aug 1, 2022

Copy link
Copy Markdown
Contributor Author

/cc @nodejs/tty

nodejs-github-bot commented Aug 15, 2022

Copy link
Copy Markdown
Collaborator

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

BridgeAR reviewed Aug 15, 2022
View reviewed changes

BridgeAR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Aug 16, 2022

Copy link
Copy Markdown
Contributor Author

@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 added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 24, 2022
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 merged commit ab89024 into nodejs:main Aug 24, 2022

nodejs-github-bot commented Aug 24, 2022

Copy link
Copy Markdown
Collaborator

Landed in ab89024

aduh95 deleted the fix-tty-TypeError branch August 25, 2022 14:06
aduh95 mentioned this pull request Aug 25, 2022
Closed
RafaelGSS pushed a commit that referenced this pull request Sep 5, 2022
tty: fix TypeError when stream is closed
8b0a4af 
Fixes: #41330
PR-URL: #43803
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
RafaelGSS mentioned this pull request Sep 5, 2022
Merged
targos pushed a commit that referenced this pull request Sep 16, 2022
tty: fix TypeError when stream is closed
21beba1 
Fixes: #41330
PR-URL: #43803
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
juanarbol mentioned this pull request Oct 11, 2022
Merged
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
tty: fix TypeError when stream is closed
be863b7 
Fixes: #41330
PR-URL: #43803
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
tty: fix TypeError when stream is closed
447ac14 
Fixes: nodejs/node#41330
PR-URL: nodejs/node#43803
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
tty: fix TypeError when stream is closed
ec4c256 
Fixes: nodejs/node#41330
PR-URL: nodejs/node#43803
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

BridgeAR BridgeAR left review comments
joyeecheung joyeecheung approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

4 participants

Back | FazBrowse Home | New Git URL