fix(hir): #1070 — instanceof narrowing breaks property read in fastify error handler - #1077
Merged
Merged
Conversation
…uest
`pre_scan_fastify_handler_params` in `crates/perry-hir/src/lower_patterns.rs`
registered the first arrow param of EVERY recognized fastify handler call
as a `("fastify", "Request")` native instance — including
`setErrorHandler(async (err, req, reply) => …)`, whose first param is the
thrown value, not a Request. With `err` tagged as Request, any user-field
read like `err.problem` lowered to a `NativeMethodCall { module: "fastify",
method: "problem", … }` whose unknown-method fall-through in
`lower_native_method_call` returns `double_literal(0.0)`, so the access
printed as `0`.
`(err as MyError).problem` worked because `expr_member.rs`'s native-instance
shape check only fires for `member.obj == Expr::Ident`; a `TsAs` cast
bypassed it and went through generic property dispatch.
Skip the err param for `setErrorHandler` so only `params[1]=request` and
`params[2]=reply` get the native-instance tags. `err.<field>` then falls
through to the generic property dispatch that reads the actual class-
instance field at runtime, restoring parity with the `as`-cast path.
Regression coverage added to `test_fastify_integration.ts` /
`run_fastify_tests.sh`: a `ProblemError` thrown from a route is
caught by `setErrorHandler`, narrowed via `instanceof`, and its
`err.problem.{status,title}` fields read correctly into the
HTTP 400 response body.
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
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.
Closes #1070.
Root cause
pre_scan_fastify_handler_params(incrates/perry-hir/src/lower_patterns.rs) registered the first arrow param of every recognized fastify handler call as a("fastify", "Request")native instance — includingapp.setErrorHandler(async (err, req, reply) => …), whose first param is the thrown value, not a Request.With
errtagged as Request, any user-field read likeerr.problemlowered to aNativeMethodCall { module: "fastify", method: "problem", … }.problemisn't in the Request method dispatch table, so thelower_native_method_callfall-through (crates/perry-codegen/src/lower_call/native.rs:2297) returneddouble_literal(0.0)— the access printed as0.(err as MyError).problemworked becauseexpr_member.rs's native-instance shape check only matchesmember.obj == Expr::Ident; aTsAscast bypassed it and went through generic property dispatch — which is why the bug report's outside-the-if access printed the full object but the inside-the-narrow bareerr.problemaccess printed0.Fix
Skip the err param for
setErrorHandlerso onlyparams[1]=requestandparams[2]=replyget the native-instance tags.err.<field>then falls through to the generic property dispatch that reads the actual class-instance field at runtime.Verification
Reproducer from the issue:
Regression coverage added in
test_fastify_integration.ts+run_fastify_tests.sh:POST /throw-problemthrows aProblemErrorinstance from a routesetErrorHandlernarrows viainstanceofand readserr.problem.{status,title}{"title":"Bad Request","status":400}bodyAll 12 fastify integration tests pass (10 pre-existing + 2 new for #1070).
No version bump or changelog change — maintainer handles that at merge time.
Test plan
cargo test -p perry-hirandcargo test -p perry-codegenpassscripts/run_fastify_tests.sh: 12/12 pass (was 10/10 pre-fix + 2 new for instanceof narrowing breaks property access in Fastify async error handler #1070)