Repository navigation
fix(core): match nested event filters when an ancestor key is missing… - #5022
Sadikdeveloper wants to merge 1 commit into
Conversation
… or null
`eventFilterMatches` returned `false` for any non-empty filter as soon as the
payload was `undefined`/`null`, without evaluating the filter itself. Because
missing nested keys recurse with an `undefined` payload, a nested `$exists:
false` filter failed whenever its parent key was missing or `null`, while the
same filter returned `true` at the top level.
A nullish payload is now matched like an empty object, so a missing payload and
a missing key take the same path. Only filters whose leaves already match a
missing key change behavior: `$exists: false`, `$isNull: false` and
`$anythingBut` now match a missing/`null`/non-object ancestor too, which is how
`$exists` behaves on nested paths elsewhere (e.g. MongoDB) and how these filters
already behaved for a missing key at the top level. Filters that require a value
(`$exists: true`, `$startsWith`, `$gt`, value arrays, `$includes`, `$not`, ...)
are unchanged.
This is the user-facing path behind `retry.fetch`'s `bodyFilter`, so a filter
like `{ error: { retryAfter: [{ $exists: false }] } }` now matches a response
body with no `error` object instead of silently skipping the retry strategy.
- core: unit tests for missing/null/non-object ancestors, deep nesting, arrays,
and guards proving positive filters don't over-match
- trigger-sdk: end-to-end tests for `retry.fetch` + `bodyFilter` against a
local server
🦋 Changeset detectedLatest commit: 71e2fea The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 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 |
|
Hi @Sadikdeveloper, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (4)
✨ Finishing Touches
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // give in MongoDB-style filters, and the same result these filters already gave for a missing key | ||
| // at the top level. Filters that need a value (`$exists: true`, `$startsWith`, `$gt`, ...) still | ||
| // don't match `undefined`. | ||
| const payloadObject = payload === undefined || payload === null ? {} : payload; |
There was a problem hiding this comment.
🟡 Missing parents match empty nested filters
When an ancestor is missing, eventFilterMatches now accepts nested {} or [] filters without an absence condition. A retry.fetch body filter like { error: {} } can retry responses without error.
Learn more
An empty nested object or array matches any present value, but the previous nullish guard rejected it when its parent was missing. The new normalization lets those empty filters succeed despite having no leaf that tests absence. This also changes SDK retry selection because getRetryStrategyForResponse uses this matcher for response bodies.
Example: With payload {} and filter { error: {} }, the previous matcher returned false; it now returns true. A retry.fetch strategy using that filter can retry a response with no error field.
Recommended fix: When traversing a missing or null ancestor, distinguish filters that actually match a missing leaf from vacuous matches produced by empty nested objects or arrays. Preserve the previous result for the latter and add tests for both empty forms.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "@trigger.dev/core": patch | ||
| --- | ||
|
|
||
| Fix `eventFilterMatches` so nested filters that assert the absence of a value match when an ancestor key is missing or `null`, instead of failing before the filter is evaluated. A missing payload is now matched like an empty object: it's how `$exists` behaves on nested paths elsewhere, and how these filters already behaved for a missing key at the top level. |
There was a problem hiding this comment.
🔍 Lead the release note with user behavior
The changeset opens with eventFilterMatches instead of the user-visible change to nested filters. Repository guidance asks release notes to lead with user behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const server = await startJsonServer((requestIndex) => | ||
| requestIndex === 1 | ||
| ? { status: 429, body: { message: "slow down" } } | ||
| : { status: 200, body: { ok: true } } | ||
| ); |
There was a problem hiding this comment.
🔍 Null response bypasses retry absence filter
A JSON null response cannot reach the new matching behavior: getRetryStrategyForResponse skips falsy bodies. The tests cover an object without error, not a null body.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
… or null
eventFilterMatchesreturnedfalsefor any non-empty filter as soon as the payload wasundefined/null, without evaluating the filter itself. Because missing nested keys recurse with anundefinedpayload, a nested$exists: falsefilter failed whenever its parent key was missing ornull, while the same filter returnedtrueat the top level.A nullish payload is now matched like an empty object, so a missing payload and a missing key take the same path. Only filters whose leaves already match a missing key change behavior:
$exists: false,$isNull: falseand$anythingButnow match a missing/null/non-object ancestor too, which is how$existsbehaves on nested paths elsewhere (e.g. MongoDB) and how these filters already behaved for a missing key at the top level. Filters that require a value ($exists: true,$startsWith,$gt, value arrays,$includes,$not, ...) are unchanged.This is the user-facing path behind
retry.fetch'sbodyFilter, so a filter like{ error: { retryAfter: [{ $exists: false }] } }now matches a response body with noerrorobject instead of silently skipping the retry strategy.retry.fetch+bodyFilteragainst a local serverCloses #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯