Skip to content

fix(compile): panic when compiled with --no-terminal flag #28823

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JasperVanEsveld
Copy link
Contributor

@JasperVanEsveld JasperVanEsveld commented Apr 10, 2025

This works towards fixing #27730 and #21091.

When compiled with the --no-terminal flag under windows there are some issues related to stdin, stdout and stderr.
This pull request fixes the panic when during bootstrap the terminal type is requested.
Previously it would return the error if the terminal was not valid, instead it now returns UNKNOWN.

The dummy stdin that is than provided instantly pushes null for some reason.
However this tries to do a debugLog which is not possible yet, as we are still initiating.
I don't see the use for this push, so I removed the line.
If it is needed for something please let me know.

This PR does not fully fix the issue yet, as there is a similar problem in deno_core's isTerminal op, see this pull request.

ry pushed a commit to denoland/deno_core that referenced this pull request Apr 11, 2025
This works towards fixing
[#27730](denoland/deno#27730) and
[#21091](denoland/deno#21091).

An error is not handled when calling `op_is_terminal`.
Ironically, this causes the application to panic when bootstrapping if
there is no terminal in windows.

This PR does not fully fix the issue yet, as there is a similar problem
in deno's code base, see this [pull
request](denoland/deno#28823).
@JasperVanEsveld
Copy link
Contributor Author

Can some one please review this one? 🙏
@ry merged a related PR in deno-core, if this one is also merged the issues can be closed.

Not sure if/who I should mention to get more eyes on it, or just have patience...

@dsherret dsherret requested a review from littledivy April 16, 2025 17:00
@dsherret dsherret added this to the 2.3.0 milestone Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants