fix(cli): suppress harmless startup warnings via pnpm patches#7820
fix(cli): suppress harmless startup warnings via pnpm patches#7820paulbalaji merged 5 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 92fc03e The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Suppressed two warnings that appear during startup in bundled packages: 1. bigint-buffer native bindings warning - comes from @solana/spl-token dependency chain, harmless as it falls back to pure JS implementation 2. node-fetch DeprecationWarning about .data property - comes from node-fetch@3.3.2 via aleo-sdk The fix moves warning suppression to the bundle header (before module loading) using --no-warnings shebang flag and an early console.warn override. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
paulbalaji
force-pushed
the
fix/cli-suppress-harmless-warnings
branch
from
d9c252b to
cce9252
Compare
paulbalaji
requested review from
antigremlin,
ltyu,
xeno097 and
yorhodes
as code owners
📝 WalkthroughWalkthroughRuntime CLI warning suppressions for bigint-buffer and node-fetch were removed and replaced by source-level patches and package.json patchedDependencies entries; CLI side-effect import that applied the runtime workaround was deleted. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.18.1).changeset/fix-cli-post-bundle-error-handler.mdmarkdownlint-cli2 v0.18.1 (markdownlint v0.38.0) 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
typescript/cli/scripts/ncc.post-bundle.mjs (1)
69-77: This catch block is broken - leftover code that'll cause a mess.There's a real problem here. The catch block references variables that don't exist:
contentis scoped to the try block and not accessible heredirnameDefandshebangare undefined (the constants areDIRNAME_SHIMandSHEBANG)If an error occurs (file not found, permission issue, etc.), this will throw a
ReferenceErrorinstead of handling it gracefully. Looks like some old code got left behind during the refactor.🐛 Proposed fix - clean up the catch block
} catch (error) { console.error('✖ Failed to patch CLI executable:', error); - if (!content.includes(dirnameDef)) { - const [, executable] = content.split(shebang); - const newContent = `${shebang}\n${dirnameDef}\n${executable}`; - await writeFile(outputFile, newContent, 'utf8'); - console.log('Adding missing __dirname definition to cli executable'); - } + process.exit(1); }This matches how the rebalancer and warp-monitor scripts handle errors.
🧹 Nitpick comments (1)
typescript/rebalancer/scripts/ncc.post-bundle.mjs (1)
10-33: Identical to the warp-monitor version - works the same way.This is the second copy of this shim logic. The implementation is correct.
If you ever get tired of maintaining the same onion layers in multiple swamps, consider extracting this
DIRNAME_SHIMand the patching logic to a shared build utility. But that's a "someday" thing - these are separate packages with separate build scripts, so keeping them isolated isn't unreasonable.
|
I wish there was a better way of doing this |
Replaced --no-warnings shebang flag and console.warn override with pnpm patches that fix the warnings at the source: - bigint-buffer@1.1.5: silence native bindings fallback warning - node-fetch@3.3.2: silence .data deprecation warning (gaxios compat) This approach won't suppress future warnings and is cleaner than runtime workarounds. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
paulbalaji
changed the title
paulbalaji
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
typescript/cli/scripts/ncc.post-bundle.mjs (1)
55-61: Fix undefined identifiers in the error handler.Right now Line 57–60 uses
dirnameDef,shebang,outputFile, andcontentwithout definitions, so the catch path throws and hides the real error.🐛 Suggested fix
async function patchCliExecutable() { - try { - let content = await readFile(OUTPUT_FILE, 'utf8'); + let content = ''; + try { + content = await readFile(OUTPUT_FILE, 'utf8'); @@ } catch (error) { console.error('✖ Failed to patch CLI executable:', error); - if (!content.includes(dirnameDef)) { - const [, executable] = content.split(shebang); - const newContent = `${shebang}\n${dirnameDef}\n${executable}`; - await writeFile(outputFile, newContent, 'utf8'); + if (content && !content.includes(DIRNAME_SHIM)) { + const [, executable = content] = content.split(SHEBANG); + const newContent = `${SHEBANG}\n${DIRNAME_SHIM}\n${executable}`; + await writeFile(OUTPUT_FILE, newContent, 'utf8'); console.log('Adding missing __dirname definition to cli executable'); } } }
paulbalaji
changed the title
♻️ Rebalancer Docker Image Built SuccessfullyImage Tags: |
🕵️ Warp Monitor Docker Image Built SuccessfullyImage Tags: |
paulbalaji
left a comment
paulbalaji
left a comment
There was a problem hiding this comment.
nuke env.ts + extraneous comments
- Deleted env.ts (only contained outdated comments) - Removed import of env.js from cli.ts - Removed explanatory comments from ncc.post-bundle.mjs files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The catch block referenced undefined variables (content, dirnameDef, shebang, outputFile) that would throw a ReferenceError if the try block failed, masking the real error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🐳 Monorepo Docker Image Built SuccessfullyImage Tags: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7820 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
2 participants
Summary
bigint: Failed to load bindingswarning from bigint-bufferDeprecationWarning: .data is not a valid RequestInit propertywarning from node-fetchApplied to all three bundled packages: cli, rebalancer, and warp-monitor.
Approach
Uses pnpm patches to fix warnings at the source, rather than runtime suppression:
patches/bigint-buffer@1.1.5.patch- Silences the console.warn when native bindings fail to loadpatches/node-fetch@3.3.2.patch- Removes the.datadeprecation warning (gaxios compatibility)This approach:
--no-warningsshebang flagconsole.warnoverridesDetails
Two harmless warnings were appearing during startup:
bigint-buffer native bindings warning
@solana/spl-token→@solana/buffer-layout-utils→bigint-buffer@1.1.5node-fetch DeprecationWarning
@hyperlane-xyz/aleo-sdk→node-fetch@3.3.2datainstead ofbodyto fetch optionsTest plan
hyperlane --helpand verify no warnings appear🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.