Skip to content

fix: let isset() and ?? see the JSON fields of a Payload - #11

Open
oniric85 wants to merge 1 commit into
mainfrom
fix/payload-isset
Open

oniric85 wants to merge 1 commit into
mainfrom
fix/payload-isset

Conversation

@oniric85

@oniric85 oniric85 commented Oct 8, 2026

Copy link
Copy Markdown
Member

Payload exposes the fields of a JSON body as properties through __get, but
it never declared __isset. PHP consults __isset, not __get, for isset(),
empty() and ??, so every one of those checks on a Payload reported the
field as missing while a plain read returned it. Client::api() tests
$result->error ?? false and so returned a JetStream error reply as a
success instead of throwing, and a caller that inspects a PubAck the same
way takes a rejected publish for an acknowledged one.

__isset now answers true when the body has the field and it is not null,
the rule isset() applies to a real property. __get is unchanged.
Client::api() throws on an error reply as it was written to do; the
functional suite is unaffected by that.

Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com

🤖 Generated with Claude Code

Payload exposes the fields of a JSON body as properties through __get, but
it never declared __isset. PHP consults __isset, not __get, for isset(),
empty() and ??, so every one of those checks on a Payload reported the
field as missing while a plain read returned it. Client::api() tests
`$result->error ?? false` and so returned a JetStream error reply as a
success instead of throwing, and a caller that inspects a PubAck the same
way takes a rejected publish for an acknowledged one.

__isset now answers true when the body has the field and it is not null,
the rule isset() applies to a real property. __get is unchanged.
Client::api() throws on an error reply as it was written to do; the
functional suite is unaffected by that.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@oniric85
oniric85 requested a review from MattiasAng October 8, 2026 22:09
@oniric85 oniric85 self-assigned this Oct 8, 2026
@oniric85
oniric85 marked this pull request as ready for review October 8, 2026 22:10
@oniric85

oniric85 commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Hi Mattias, I opened this small PR.

Payload has __get but no __isset, so isset(), empty() and ?? on its JSON fields always say "not set". Client::api() checks $result->error ?? false, so a JetStream error reply comes back as a success, and JetStreamAck::fromRawAck() in php-event-client has the same problem: a rejected publish looks acknowledged. We hit it on EC-4594 ticket, where ICF signed events got lost with "handled successfully" in the logs.

The PR adds __isset with the same rule PHP applies to real properties (set and not null), plus two tests. The functional suite is unchanged.

Could you review it when you have a moment? And, would a tag with just this fix be possible, or does it have to wait for other work? We need to bump php-event-client after it and then eConsent.

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.

1 participant