docs(errors): document RejectionStanza and ServerError's response field - #499
Conversation
…ield Reflects oxidezap/whatsapp-rust#1257: IqError::ServerError gains a response: RejectionStanza field carrying the rejection stanza verbatim, and From<wacore::request::IqError> is replaced by IqError::from_response.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
📝 WalkthroughWalkthroughThe API documentation records that ChangesIQ error API
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|
| Filename | Overview |
|---|---|
| api/errors.mdx | Documents the retained rejection stanza and associated migration guidance; no eligible follow-up defect was established. |
Reviews (3): Last reviewed commit: "docs(errors): fully qualify NodeBuilder ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/errors.mdx`:
- Line 14: Rewrite the migration guidance in api/errors.mdx at lines 14, 154,
173-175, 615, and 744-777 using concise active-voice, second-person sentences
with one idea per sentence: state directly what you receive in
IqError::ServerError; separately describe storage, parsed fields, and
allocation; separately explain node access, logging, and privacy; address
callers who match or construct ServerError; and separate matching compatibility,
fixture construction, and conversion migration steps.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 576613ed-cc6e-43d1-93a5-ce38ac946f02
📒 Files selected for processing (1)
api/errors.mdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 130d945a86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Added in PR #1100: `wacore::request::IqError` gained public `is_timeout()` (`true` only for `Timeout`) and `is_transport_unavailable()` (`true` for `NotConnected`, `Disconnected`, and `InternalChannelClosed`) methods, each an exhaustive match so a future variant has to be classified rather than silently defaulting to `false`. `whatsapp_rust::request::IqError` (the crate-level type shown above, with the extra `Socket`/`EncryptSend`/`ClientState`/`EncodeError`/`ParseError` variants) makes the same judgement internally but does not expose it publicly — go through [`ErrorChainExt`](#error-chain-recovery) instead, which handles both types. | ||
|
|
||
| Added in PR #1257: `ServerError` carries `response: RejectionStanza` — see [`RejectionStanza`](#rejectionstanza) above and the [migration note](#from-pr-1257-servererror-carries-the-rejection-stanza) below. Matching with `..` is unaffected by this field; only hand-constructing the variant (mainly test fixtures) needs updating. |
There was a problem hiding this comment.
ServerError patterns
Downstream matches that explicitly bind all four former fields without .. also need updating: adding response makes those patterns fail with E0027. Saying only hand-constructed variants need changes omits this migration case; advise callers to add .. or bind the new response field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed in 3084018. Added a note (and before/after example) that a ServerError { code, text, error_type, backoff } match without .. now fails E0027 once response is added — not just hand-constructed variants.
Generated by Claude Code
| } | ||
| ``` | ||
|
|
||
| A fixture with no real wire response can build one directly: decode a hand-built `<iq type="error">` through `OwnedNodeRef::new`, wrap it in `Arc`, and convert. |
There was a problem hiding this comment.
OwnedNodeRef::new consumes unpacked node bytes, whereas every marshal* function used to encode a hand-built Node includes a leading format byte. A fixture that follows this instruction by marshalling its <iq> and passing that buffer directly to new therefore supplies the wrong input; document the required unpack step or provide a complete construction example.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and confirmed against advanced/binary-protocol.mdx's existing docs for unpack/marshal. Fixed in 3084018: the fixture snippet now marshals the node, then runs it through wacore_binary::util::unpack to strip the format byte before handing the result to OwnedNodeRef::new, with a link to the format-byte section for context.
Generated by Claude Code
| } | ||
| ``` | ||
|
|
||
| `ServerRejection`'s four fields cover what WA Web's own `parseIqResponse` reads off an error; `RejectionStanza` is the escape hatch for everything that parser (and this crate's) leaves unread — further `<iq>`/`<error>` attributes, `<error>` children such as XMPP application-condition elements, and the raw bytes, which are the only faithful material for logging or replaying a rejection. `Deref<Target = OwnedNodeRef>` (see [`OwnedNodeRef`](/advanced/binary-protocol#ownednoderef-yoke-zero-copy)) keeps every node accessor reachable directly on the wrapper — `response.tag()`, `response.attrs()`, `response.get_optional_child(...)`, or `response.get()` for the underlying `NodeRef`. |
There was a problem hiding this comment.
This paragraph combines parser behavior, preserved data categories, logging and replay implications, dereference mechanics, and accessor examples into two dense sentences. Split these independent ideas into concise sentences so the new reference follows the project's one-idea-per-sentence requirement.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same style question CodeRabbit raised on this PR (line 14) — skipping for the same reason: the surrounding unchanged content on this page is consistently dense, third-person, rationale-heavy prose (e.g. the http_status() bullet above, or the ServerRejection line), and I matched that established voice deliberately rather than switching tone only for the new sections. CodeRabbit agreed and withdrew its equivalent finding. Happy to revisit if a maintainer wants the whole page's voice changed.
Generated by Claude Code
…unpack step - Note that a ServerError match binding all four former fields without `..` now fails E0027 once `response` is added, not just hand-constructed variants. - Show the required unpack() step when building a fixture from marshal() output for OwnedNodeRef::new, which expects the format byte already stripped.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Matches the fully-qualified style already used for marshal/unpack in the same snippet, per cubic's review — the bare NodeBuilder reference wasn't importable as written.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Documentation-only update to api/errors.mdx explaining an API change from another PR; no behavior, security, or operational tradeoffs are introduced.
Re-trigger cubic
1 participant
Summary
Documents oxidezap/whatsapp-rust#1257, which changed
IqError::ServerErrorto carry the rejection stanza verbatim instead of discarding it after extracting four summary fields.IqError::ServerErrorgainsresponse: RejectionStanza— thetype="error"<iq>stanza the receive path already decoded, handed over the same way atype="result"response is. Matching with..is unaffected; constructing the variant by hand now needs the stanza.RejectionStanza, wrappingArc<OwnedNodeRef>withas_arc()/into_arc(),Deref<Target = OwnedNodeRef>, and aDebugthat prints only the tag (<iq>) to avoid leaking attribute/child values — such as a JID — into{e:?}logging on the connect path.From<wacore::request::IqError> for IqErroris replaced byIqError::from_response(err, &response), since a bareFromhas no response to attach.Changes
api/errors.mdx:### RejectionStanzatype-definition section (placed next to### ServerRejection, which it complements).IqErrorbase-type code block updated with the newresponse: RejectionStanzafield, plus an "Added in PR #1257" note.ServerErrorcarries the rejection stanza" section in the migration guide, covering both the field addition and theFrom→from_responsereplacement.No other page referenced the
ServerErrorfield list or the removedFromimpl, so no other files needed changes. Changelog entries are intentionally left untouched.Generated by Claude Code
Summary by cubic
Docs update to explain the new
response: RejectionStanzaonIqError::ServerErrorand the move toIqError::from_response. Also fixes the fixture snippet by fully qualifyingwacore_binary::builder::NodeBuilder...are unchanged...or bindresponse(E0027).IqError::ServerError, includeresponse: RejectionStanza(Arc<OwnedNodeRef>via.into()).From<wacore::request::IqError>withIqError::from_response(err, &response).wacore_binary::builder::NodeBuilder, and rununpack()beforeOwnedNodeRef::new.Written for commit bfa6d8e. Summary will update on new commits.
Summary by CodeRabbit