Skip to content

ci: fix silently failing storybook build#12041

Closed
Khoyo wants to merge 1 commit into
devfrom
yk/fix-silently-failing-storybook
Closed

ci: fix silently failing storybook build#12041
Khoyo wants to merge 1 commit into
devfrom
yk/fix-silently-failing-storybook

Conversation

@Khoyo
Copy link
Copy Markdown
Contributor

@Khoyo Khoyo commented Jun 6, 2025

Storybook tries to read stdin to when it fails (to get telemetry opt-in). If stdin is closed, it fails succesfully... Setting CI=true disable this behavior (and other similar ones), and it's a good practice to have it set anyway.

See https://github.com/OpenRailAssociation/osrd/actions/runs/15485923418/job/43600490309

@Khoyo Khoyo requested a review from a team as a code owner June 6, 2025 21:37
@Khoyo Khoyo force-pushed the yk/fix-silently-failing-storybook branch from 7ccd255 to 055e744 Compare June 6, 2025 21:39
Storybook tries to read stdin to when it fails (to get telemetry
opt-in). If stdin is closed, it fails succesfully... Setting CI=true
disable this behavior (and other similar ones), and it's a good practice
to have it set anyway.

Signed-off-by: Younes Khoudli <younes.khoudli@epita.fr>
@Khoyo Khoyo force-pushed the yk/fix-silently-failing-storybook branch from 055e744 to 3c73503 Compare June 6, 2025 21:42
Copy link
Copy Markdown
Contributor

@Synar Synar left a comment

Choose a reason for hiding this comment

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

Nice find!

(I doubt generate licenses will fail any time soon, but I doubt giving it CI=true will cause any problem either ^^)

@emersion
Copy link
Copy Markdown
Member

emersion commented Jun 6, 2025

This sounds a bit fragile to me, setting CI=true in a Docker image used for local development and rely on its implicit behavior…

Would STORYBOOK_DISABLE_TELEMETRY=1 work as an alternative? Would be more explicit and targeted.

(We should really report this as a storybook bug…)

@emersion emersion self-requested a review June 10, 2025 08:14
@Khoyo
Copy link
Copy Markdown
Contributor Author

Khoyo commented Jun 11, 2025

This sounds a bit fragile to me, setting CI=true in a Docker image used for local development and rely on its implicit behavior…

Well, this image is used in the CI, and is in fact the one used in production. (And I wouldn't expect most people to build it to dev on the frontend, this is what dev-front is for)

Would STORYBOOK_DISABLE_TELEMETRY=1 work as an alternative? Would be more explicit and targeted.

It would, but wouldn't catch other potential mishaps in other components. Maybe we should set both, wdyt?

(We should really report this as a storybook bug…)

We should indeed. I didn't dig deeper to see what the exact culprit was though.

@Synar
Copy link
Copy Markdown
Contributor

Synar commented Jun 11, 2025

I'm working on a possible storybook side fix, but I'm unsure if it will be accepted

@Synar
Copy link
Copy Markdown
Contributor

Synar commented Jun 15, 2025

See storybookjs/storybook#31781

There may be something deeper too.

@emersion
Copy link
Copy Markdown
Member

Well, this image is used in the CI, and is in fact the one used in production. (And I wouldn't expect most people to build it to dev on the frontend, this is what dev-front is for)

It's used by default when building OSRD.

CI would be correct to set as an env variable from the GitHub Action manifest, but not from the Dockerfile.

We want the build to fail as expected outside of CI, too.

It would, but wouldn't catch other potential mishaps in other components. Maybe we should set both, wdyt?

The logic to detect the CI env var is open-coded specifically for the telemetry. Just like isatty was forgotten here, checking for CI could be forgotten elsewhere too. So I don't think it buys a lot of safety.

@Synar
Copy link
Copy Markdown
Contributor

Synar commented Jun 23, 2025

See storybookjs/storybook#31781

There may be something deeper too.

This was merged, as soon as it gets released I'll open a bump pr

@Khoyo Khoyo closed this Jun 24, 2025
@Khoyo Khoyo deleted the yk/fix-silently-failing-storybook branch December 15, 2025 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants