Skip to content

Downstream issues emerging after forge config output changes #9240

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

Closed
2 tasks done
GlacierWalrus opened this issue Oct 31, 2024 · 3 comments · Fixed by #9243
Closed
2 tasks done

Downstream issues emerging after forge config output changes #9240

GlacierWalrus opened this issue Oct 31, 2024 · 3 comments · Fixed by #9243
Labels
A-compatibility Area: compatibility T-bug Type: bug T-to-investigate Type: to investigate

Comments

@GlacierWalrus
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (9d74675 2024-10-31T00:20:54.778420069Z)

What command(s) is the bug in?

forge config --json

Operating System

Linux

Describe the bug

I'm using a typescript library which runs forge config --json to acquire the forge config.

This library started crashing out of the blue recently when my build system was executing hardhat compile. The error looked like this:

An unexpected error occurred:
SyntaxError: Unexpected non-whitespace character after JSON at position 5069 (line 186 column 1)
    at JSON.parse (<anonymous>)

I wrote the buffer to file and inspected it

❯ hexdump dump.json -C | tail -n 5
000013c0  73 65 2c 0a 20 20 22 74  72 61 6e 73 61 63 74 69  |se,.  "transacti|
000013d0  6f 6e 5f 74 69 6d 65 6f  75 74 22 3a 20 31 32 30  |on_timeout": 120|
000013e0  2c 0a 20 20 22 65 6f 66  22 3a 20 66 61 6c 73 65  |,.  "eof": false|
000013f0  0a 7d 0a 1b 5b 30 6d                              |.}..[0m|
000013f7

and you can see an ANSI control code at the end of the output.

Given the nature of my setup I found the best workaround was to set NO_COLOR=1 prior to running hardhat

This line might be the culprit?

I'd suggest perhaps checking if the output is a tty before writing ANSI codes.

@GlacierWalrus GlacierWalrus added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Oct 31, 2024
@zerosnacks zerosnacks added A-compatibility Area: compatibility T-to-investigate Type: to investigate and removed T-needs-triage Type: this issue needs to be labelled labels Nov 1, 2024
@zerosnacks
Copy link
Member

Hi @GlacierWalrus thanks for flagging this

I think the most practical way to fix this would be to ensure --json is always ran in a NoColor mode.

Related: #8794 which plans to add a global --json flag which would make this easy to enforce

Shell::new_with(self.color.unwrap_or_default(), verbosity)

cc @DaniPopes

@GlacierWalrus
Copy link
Author

That makes more sense, thank you for explaining your codebase to me.

@DaniPopes
Copy link
Member

DaniPopes commented Nov 1, 2024

This function should maybe check is_tty() on auto

actually auto shouldn't happen here, that line is not the culprit

fn supports_color(choice: anstream::ColorChoice) -> bool {
match choice {
anstream::ColorChoice::Always |
anstream::ColorChoice::AlwaysAnsi |
anstream::ColorChoice::Auto => true,
anstream::ColorChoice::Never => false,
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: compatibility T-bug Type: bug T-to-investigate Type: to investigate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants