You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(metascraper): log rule values without a string form - #890
The per-rule debug line in packages/metascraper/src/get-data.js now prints primitive values as they are and anything else as its type (object, function, symbol).
Why
The message interpolated the value into a template literal outside the try, and it is built even when debug is off. A null-prototype object, which is what an evaluate rule such as window.location returns in microlink-api, has no toPrimitive, so it threw TypeError: Cannot convert object to primitive value and rejected the whole extraction. That surfaced as EFATAL 500s in the 2026-11-02 microlink-api unexpected-errors export (data.ajax.evaluate=window.location). Symbols and null-prototype functions threw the same way.
Printing the type instead of serializing the value means the line can't throw and never builds a large string on every extraction. An earlier util.inspect draft cost 52ms per extraction on a 100k-key object and failed on a throwing [util.inspect.custom].
Measurable outcome
microlink-api "Cannot convert object to primitive value" EFATALs (2 in the last 30 days) go to 0 once the patch release is picked up through ~5.58.5.
Tests
New unit test a rule value without a string form is returned as is: null-prototype object, cyclic object, throwing inspect.custom, revoked proxy, symbol, null-prototype function. It fails on master with the exact production stack. NODE_PATH=.. TZ=UTC npx ava in packages/metascraper: 121 passed with DEBUG unset and with DEBUG=metascraper*.
Low Risk
Scoped change to debug logging only; extraction results are unchanged and the fix removes a throw path during metadata collection.
Overview
Fixes a production crash where the per-rule duration debug line in get-data.js interpolated rule values into a template string even when debug was off, so null-prototype objects, symbols, and similar values threw "Cannot convert object to primitive value" and failed the whole extraction.
The log now uses formatValue: primitives and null are printed literally; everything else is logged as its typeof (e.g. object, function, symbol) so the line never coerces non-printable values. Returned metadata is unchanged.
Unit coverage adds cases for awkward rule outputs (cyclic objects, revoked proxies, throwing inspect.custom, etc.) and asserts both metadata and captured metascraper:get-data log lines.
Reviewed by Cursor Bugbot for commit a4229aa. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Debug output now displays primitive values directly and represents other values by type, including complex objects, symbols, and functions. Rule values continue to be preserved in metadata, including values without a string representation.
The per-rule debug line interpolated the value into a template literal
outside the try block. A null-prototype object (what an `evaluate` rule
such as `window.location` returns) has no toPrimitive, so it threw
"Cannot convert object to primitive value" and rejected the whole
extraction. Symbols and null-prototype functions threw the same way.
The message is built even when debug is off.
The line now prints primitive values and the type of anything else, so
it never throws and never serializes a large value on every extraction.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rule-duration log messages now format normalized values before logging. Unit tests cover primitive values and rule results such as cyclic objects, symbols, functions, and revoked proxies.
Changes: Rule value logging
Layer / File(s)
Summary
Format rule values for duration logs packages/metascraper/src/get-data.js, packages/metascraper/test/unit/index.js
The formatter preserves selected primitive values and returns the type name for other values. Duration logs use the formatter. Unit tests cover rule results without a standard string form.
No concrete logging or metadata regression remains identified; the change is mergeable after normal checks.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "fix(metascraper): log rule values without a string form" states the main change. The PR changes per-rule debug output in packages/metascraper/src/get-data.js so that null and primitive valu…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "fix(metascraper): log rule values without a string form" states the main change. The PR changes per-rule debug output in packages/metascraper/src/get-data.js so that null and primitive values are logged as-is and other values are logged by type. The title names the affected package and the fix. It is specific and concise. The title is 55 characters and 8 words, which fits a short single-sentence title. The test additions in packages/metascraper/test/unit/index.js cover the same behavior.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
The suite only checked that unusual values came back, so logging a string, number, boolean, bigint, or null as its type still passed. Assert the logged text for each, and drop undefined, which never reaches the logger.
The reason will be displayed to describe this comment to others. Learn more.
Additional comments: 1
packages/metascraper/test/unit/index.js: 1
8-10: Trivial · Maintainability & Code Quality · Low value
Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Declare debug as a dependency, or note why the transitive resolution is safe.
The test resolves debug from the install path of debug-logfmt. This makes sure that the test controls the same debug instance that the library uses. The approach is sound. It depends on debug being a dependency of debug-logfmt, which package.json does not declare directly. The static-analysis warning about a non-literal require is a false positive here. The path is not user-controlled.
Add a short comment above the require to explain why debug resolves through debug-logfmt. This prevents a later cleanup from replacing it with a direct require('debug'). A direct require('debug') could load a different instance and silently stop capturing logs.
Source: Linters/SAST tools
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: abbac3a2-50e4-4c35-8be6-87377112b086
Commits
Reviewing files that changed from the base of the PR and between f3c3a96 and a4229aa.
Files selected for processing: 2
packages/metascraper/src/get-data.js
packages/metascraper/test/unit/index.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
The per-rule debug line in
packages/metascraper/src/get-data.jsnow prints primitive values as they are and anything else as its type (object,function,symbol).Why
The message interpolated the value into a template literal outside the
try, and it is built even when debug is off. A null-prototype object, which is what anevaluaterule such aswindow.locationreturns in microlink-api, has notoPrimitive, so it threwTypeError: Cannot convert object to primitive valueand rejected the whole extraction. That surfaced as EFATAL 500s in the 2026-11-02 microlink-api unexpected-errors export (data.ajax.evaluate=window.location). Symbols and null-prototype functions threw the same way.Printing the type instead of serializing the value means the line can't throw and never builds a large string on every extraction. An earlier
util.inspectdraft cost 52ms per extraction on a 100k-key object and failed on a throwing[util.inspect.custom].Measurable outcome
microlink-api
"Cannot convert object to primitive value"EFATALs (2 in the last 30 days) go to 0 once the patch release is picked up through~5.58.5.Tests
New unit test
a rule value without a string form is returned as is: null-prototype object, cyclic object, throwinginspect.custom, revoked proxy, symbol, null-prototype function. It fails on master with the exact production stack.NODE_PATH=.. TZ=UTC npx avainpackages/metascraper: 121 passed withDEBUGunset and withDEBUG=metascraper*.🤖 Generated with Claude Code
Note
Low Risk
Scoped change to debug logging only; extraction results are unchanged and the fix removes a throw path during metadata collection.
Overview
Fixes a production crash where the per-rule duration debug line in
get-data.jsinterpolated rule values into a template string even when debug was off, so null-prototype objects, symbols, and similar values threw "Cannot convert object to primitive value" and failed the whole extraction.The log now uses
formatValue: primitives andnullare printed literally; everything else is logged as itstypeof(e.g.object,function,symbol) so the line never coerces non-printable values. Returned metadata is unchanged.Unit coverage adds cases for awkward rule outputs (cyclic objects, revoked proxies, throwing
inspect.custom, etc.) and asserts both metadata and capturedmetascraper:get-datalog lines.Reviewed by Cursor Bugbot for commit a4229aa. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit