Skip to content

chore: print more lunaria generation logs#2848

Open
alexdln wants to merge 1 commit into
npmx-dev:mainfrom
alexdln:chore/print-lunaria-logs
Open

chore: print more lunaria generation logs#2848
alexdln wants to merge 1 commit into
npmx-dev:mainfrom
alexdln:chore/print-lunaria-logs

Conversation

@alexdln
Copy link
Copy Markdown
Member

@alexdln alexdln commented Jun 3, 2026

🔗 Linked issue

Related #2785

🧭 Context

The error only in mentioned issue occurs only in canary. We have logic that outputs logs that can help understand what exactly happened, but these logs are only printed in release, not canary. To understand what exactly happened, enabling logs in canary would help

I'm almost sure that the problem is because lunaria, for some reason, can't process git data in the main branch (it definitely didn't generate the file and it definitely crashed here, but without logs it's hard to understand why)

Overall, I can't find a reason why we should ignore logs, so I want us to display them

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Jun 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Jun 3, 2026 7:59pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Jun 3, 2026 7:59pm
npmx-lunaria Ignored Ignored Jun 3, 2026 7:59pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Improved subprocess output visibility during the module's build-time TypeScript transformation, making build process output directly visible.
    • Enhanced error handling in the build process, with better error reporting and environment-specific error management across development, release, and canary environments.

Walkthrough

The module's build-time TypeScript transform subprocess now displays its output directly via inherited stdio. Error handling in the build hook is expanded to treat canary environments the same as dev and release, with non-matching environments logging errors to the console rather than rethrowing them.

Changes

Build Process Updates

Layer / File(s) Summary
Build subprocess I/O and error handling
modules/lunaria.ts
The execSync invocation is configured with stdio: 'inherit' to make subprocess output directly visible. Error handling in the nitro:build:before hook catch block is updated to include canary in the environment check for rethrowing, and errors in other environments are logged via console.error(e) instead of being thrown.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enabling additional Lunaria generation logs through stdio inheritance and expanded error logging.
Description check ✅ Passed The description is directly related to the changeset, explaining the context (issue #2785), the problem (logs only printed in release), and the solution (enable logs in canary).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
modules/lunaria.ts (1)

40-40: 💤 Low value

Consider updating the inline comment to reflect canary.

The logic correctly adds canary to the throw condition, which aligns with the PR objective. However, the comment on lines 37-38 now reads "In CI, only throw if building for production" but the code throws in both release and canary builds. Consider updating the comment to be more accurate.

📝 Suggested comment update
-          // Always throw in local dev.
-          // In CI, only throw if building for production.
+          // Always throw in local dev.
+          // In CI, throw for release and canary builds to catch errors early.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/lunaria.ts` at line 40, The inline comment above the environment
check is now inaccurate — update the comment that precedes the condition using
the env variable to reflect that CI should only throw for production-like
builds, including both "release" and "canary" (not just "production"); locate
the condition containing if (env === 'dev' || env === 'release' || env ===
'canary') and edit the comment immediately above it to mention "release and
canary" (or "production-like builds") so it matches the actual logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@modules/lunaria.ts`:
- Line 40: The inline comment above the environment check is now inaccurate —
update the comment that precedes the condition using the env variable to reflect
that CI should only throw for production-like builds, including both "release"
and "canary" (not just "production"); locate the condition containing if (env
=== 'dev' || env === 'release' || env === 'canary') and edit the comment
immediately above it to mention "release and canary" (or "production-like
builds") so it matches the actual logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 02059b4e-9842-46de-9018-8c96cae28efe

📥 Commits

Reviewing files that changed from the base of the PR and between 7ee8f4c and 8832b63.

📒 Files selected for processing (1)
  • modules/lunaria.ts

Copy link
Copy Markdown
Contributor

@trueberryless trueberryless left a comment

Choose a reason for hiding this comment

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

I hope this doesn't cost Daniel too much 😅

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Jun 3, 2026

@trueberryless It should be one line for success and probably ~10 lines for failure. So it's nothing compared to the volume of logs from 100k users 😅

@trueberryless
Copy link
Copy Markdown
Contributor

@trueberryless It should be one line for success and probably ~10 lines for failure. So it's nothing compared to the volume of logs from 100k users 😅

Ah true, Lunaria only runs once at build, right? So it's not per request. I confused it.

@alexdln
Copy link
Copy Markdown
Member Author

alexdln commented Jun 3, 2026

Yes, and generates static file - /lunaria/status.json

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