{{ message }}
Preserve printf-style format substitution when piping server logs to browser console#479
Open
mvanhorn wants to merge 1 commit into
Open
Preserve printf-style format substitution when piping server logs to browser console#479mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/devtools-vite/src/virtual-console.ts`:
- Around line 321-324: The preserve-format detection in getConsoleFormatIndex is
too permissive and can treat literal percent text like “100%c” as a format
string when there are no matching substitution arguments. Update the logic
around getConsoleFormatIndex and the related preserve-format path to only
preserve formatting when the number of actual console placeholders is supported
by the available trailing args, so literal % sequences are not reclassified
after prefix injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37abba98-2a17-4807-b0ce-73c14e43479c
📒 Files selected for processing (3)
.changeset/fresh-mails-bow.mdpackages/devtools-vite/src/virtual-console.test.tspackages/devtools-vite/src/virtual-console.ts
Comment on lines
+321
to
+324
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid classifying literal % sequences as format substitutions.
getConsoleFormatIndex currently enables the preserve-format branch even when there are no substitution arguments. Literal messages like "CPU at 100%c" can then be treated as format strings and rendered incorrectly after prefix injection. Gate preservation by placeholder count vs available trailing args.
Suggested fix
- function hasConsoleFormatSubstitution(arg) {
- return typeof arg === 'string' && /(^|[^%])%[sdifocOj]/.test(arg);
- }
+ function getConsoleFormatPlaceholderCount(arg) {
+ if (typeof arg !== 'string') return 0;
+ var matches = arg.match(/(^|[^%])%[sdifocOj]/g);
+ return matches ? matches.length : 0;
+ }
function isEnhancedLogPrefix(arg) {
return typeof arg === 'string' &&
arg.indexOf('LOG') !== -1 &&
arg.indexOf(String.fromCharCode(10) + ' ' + String.fromCharCode(8594) + ' ') !== -1;
}
+ function hasResolvableConsoleFormatSubstitution(args, formatIndex) {
+ var placeholderCount = getConsoleFormatPlaceholderCount(args[formatIndex]);
+ return placeholderCount > 0 && (args.length - (formatIndex + 1)) >= placeholderCount;
+ }
+
function getConsoleFormatIndex(args) {
- if (hasConsoleFormatSubstitution(args[0])) return 0;
- return isEnhancedLogPrefix(args[0]) && hasConsoleFormatSubstitution(args[1]) ? 1 : -1;
+ if (hasResolvableConsoleFormatSubstitution(args, 0)) return 0;
+ return isEnhancedLogPrefix(args[0]) && hasResolvableConsoleFormatSubstitution(args, 1) ? 1 : -1;
}Also applies to: 334-336
🤖 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 `@packages/devtools-vite/src/virtual-console.ts` around lines 321 - 324, The
preserve-format detection in getConsoleFormatIndex is too permissive and can
treat literal percent text like “100%c” as a format string when there are no
matching substitution arguments. Update the logic around getConsoleFormatIndex
and the related preserve-format path to only preserve formatting when the number
of actual console placeholders is supported by the available trailing args, so
literal % sequences are not reclassified after prefix injection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Server log messages that use console format strings (
%s,%d,%c, etc.) are now substituted correctly when forwarded to the browser devtools console, including the default enhanced-logs path where the source-location prefix sits before the user's arguments. Previously the format string was passed through as a plain argument, so substitutions were not applied and the raw%stokens appeared in the panel.The SSE bridge that forwards server-side
console.log/console.errordid not detect format strings in all cases: with enhanced logs enabled (the default),enhanceConsoleLogprepends aLOG /path:line:colprefix, so the real format string was no longer the first argument and the substitution path was skipped. This now accounts for the enhanced-log prefix, the direct SSE path, and the%c/empty-argument edge cases. Closes #435.✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
%sand%csubstitutions.Bug Fixes