Skip to content

Add regression test for Object.getOwnPropertyDescriptors with JSG_INSPECT_PROPERTY#6538

Merged
harrishancock merged 1 commit intomainfrom
harris/2026-04-09-getownpropertydescriptors-regression-test
Apr 9, 2026
Merged

Add regression test for Object.getOwnPropertyDescriptors with JSG_INSPECT_PROPERTY#6538
harrishancock merged 1 commit intomainfrom
harris/2026-04-09-getownpropertydescriptors-regression-test

Conversation

@harrishancock
Copy link
Copy Markdown
Collaborator

Regression test for #4147.

Object.getOwnPropertyDescriptors() on a JSG prototype with JSG_INSPECT_PROPERTY used to throw "Illegal invocation" because registerInspectProperty() used SetNativeDataProperty on the prototype. V8 would invoke the native getter callback during descriptor retrieval to produce a data descriptor value, and the callback's HasInstance check would fail because the prototype object is not an instance of the type.

Fixed by PR #6003 (c43c98a), which changed registerInspectProperty() to use SetAccessorProperty instead.

Assisted-by: Claude

…PECT_PROPERTY

Regression test for #4147.

Object.getOwnPropertyDescriptors() on a JSG prototype with
JSG_INSPECT_PROPERTY used to throw "Illegal invocation" because
registerInspectProperty() used SetNativeDataProperty on the prototype.
V8 would invoke the native getter callback during descriptor retrieval
to produce a data descriptor value, and the callback's HasInstance check
would fail because the prototype object is not an instance of the type.

Fixed by PR #6003 (c43c98a), which changed registerInspectProperty()
to use SetAccessorProperty instead.

Assisted-by: Claude
@harrishancock harrishancock requested a review from jasnell April 9, 2026 12:41
@harrishancock harrishancock requested review from a team as code owners April 9, 2026 12:41
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 9, 2026

Good - I can confirm at line 1537 that registerInspectProperty() uses SetAccessorProperty (the fix from PR #6003). The test is correctly targeting the regression.

Now I have a thorough understanding of the PR. This is a straightforward, well-structured regression test that:

  1. Adds a minimal InspectableObject type with JSG_INSPECT_PROPERTY
  2. Tests that Object.getOwnPropertyDescriptors() on the prototype doesn't throw (the exact regression from Object.getOwnPropertyDescriptors when trying to clone stream #4147)
  3. Follows the existing file conventions (struct definitions, context, isolate declaration, Evaluator, expectEval)
  4. Tests both prototype and instance cases

The test is correct, follows existing patterns, and has no issues. There are no security, safety, backward compatibility, or code quality concerns.

LGTM

github run

@harrishancock harrishancock merged commit f7ae6bc into main Apr 9, 2026
25 checks passed
@harrishancock harrishancock deleted the harris/2026-04-09-getownpropertydescriptors-regression-test branch April 9, 2026 13:43
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