Skip to content

fix(events): make ReactionUpdateEvent counters/total_count optional#69

Open
sau7777 wants to merge 1 commit into
MaxApiTeam:mainfrom
sau7777:fix/reaction-counters-optional
Open

fix(events): make ReactionUpdateEvent counters/total_count optional#69
sau7777 wants to merge 1 commit into
MaxApiTeam:mainfrom
sau7777:fix/reaction-counters-optional

Conversation

@sau7777

@sau7777 sau7777 commented Jun 20, 2026

Copy link
Copy Markdown

Problem

ReactionUpdateEvent declares counters and total_count as required. But when a reaction is removed, the server emits NOTIF_MSG_REACTIONS_CHANGED (opcode 155) without a counters field (and total_count 0):

{"chatId": 531302608, "totalCount": 0, "messageId": "116781851510470562"}

This raises a ValidationError in EventMapper.mapRuntimeError: Failed to dispatch inbound frame, so reaction-removal events never reach on_reaction_update, and every removal spams the logs with a traceback:

ReactionUpdateEvent
counters
  Field required [type=missing, input_value={'chatId': ..., 'totalCount': 0, 'messageId': ...}]

Observed live on a linked-device session (real account, June 2026): add a reaction → OP155 with counters; remove it → OP155 without counters → crash.

Fix

Default counters to an empty list and total_count to 0, so removal frames validate cleanly — an empty counters simply signals "reaction removed".

Notes

  • Backward compatible: add/update frames (with counters) are unaffected.
  • Verified against the real removal payload (validates, counters == []) and a normal add payload (parses counters as before).

Summary by CodeRabbit

  • Bug Fixes
    • Improved reaction event handling by adding sensible default values for reaction counters and total count, ensuring the system gracefully handles reaction data without requiring all fields to be explicitly provided.

On reaction removal the server sends NOTIF_MSG_REACTIONS_CHANGED (OP155)
without `counters` (and total_count 0). The model marked both as required,
so removal frames raised a ValidationError and failed to dispatch. Default
counters to an empty list and total_count to 0 so removal events validate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bc582e4-c047-4f58-9cae-4f7a2e2c06a0

📥 Commits

Reviewing files that changed from the base of the PR and between 8c40b71 and 2bf6bf8.

📒 Files selected for processing (1)
  • src/pymax/types/events/reaction.py

📝 Walkthrough

Walkthrough

In src/pymax/types/events/reaction.py, Field is imported from pydantic and ReactionUpdateEvent's counters field is given a default of Field(default_factory=list) while total_count defaults to 0, making both fields optional at instantiation.

Changes

ReactionUpdateEvent optional field defaults

Layer / File(s) Summary
Optional field defaults for ReactionUpdateEvent
src/pymax/types/events/reaction.py
Adds Field import from pydantic; changes counters to Field(default_factory=list) and total_count to = 0, with updated docstring reflecting optional semantics.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A rabbit once needed no count,
For reactions could mount or not mount.
With a zero in place,
And a list full of grace,
Defaults now remove the old doubt!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making ReactionUpdateEvent counters/total_count optional with default values.
Description check ✅ Passed The description provides clear problem statement, root cause, fix explanation, and backward compatibility notes; follows the template structure with comprehensive details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ink-developer

Copy link
Copy Markdown
Collaborator

Спасибо за PR но есть пару недочетов
Описал их в ревью

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