A nosub with an error deletes the subscription entry; one without keeps it

Open
  • Is something wrong?
#360 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Needs clarification
Activity status
Quiet
Tech stack
typescript
Domain
api, distributed-systems

Research direction

Start with ADR-0004 and trace the DDP response handling used by subscribe and subscribeAll, focusing on nosub messages with and without an error field. Compare the two bookkeeping paths against the reported driver behavior; done means the intended behavior is decided and either documented and pinned or consistently implemented.

Written by the indexing model from the issue text.

Description

needs-triage

What happens

The SDK never inspects msg === 'nosub'. The only discriminator on a DDP response is the presence of an error field, so the two refusal shapes a server can send take opposite paths through the subscription bookkeeping:

  • A nosub carrying an error rejects the send with a DDPError, and subscribe forgets the entry under that id (the ADR-0004 rule).
  • A nosub without an error settles the wait as a success with no subs, so subscribe writes nothing new but also forgets nothing — an existing entry survives, unconfirmed.

Measured against the real driver: after an errored nosub both recorded entries were gone (subscribeAll re-sent nothing); after an errorless one both survived.

The real Rocket.Chat server sends the destructive shape. Meteor itself does not check auth on sub; the publication does, and Rocket.Chat's stream-notify-user rule is Boolean(this.userId) && this.userId === userId, which throws not-allowed and produces { msg: 'nosub', id, error: {...} }. The EE ddp-streamer behaves the same.

Question

Should the two shapes keep taking opposite paths? If the distinction is deliberate it deserves to be stated and pinned; if it is an accident of where the error field is read, the paths should be unified.

Dominant language
TypeScript
Stars
146
Forks
101
No merged PRs in 30d

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from RocketChat/Rocket.Chat.js.SDK

All issues in RocketChat/Rocket.Chat.js.SDK

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.