Deadlines are computed at seven call sites, so nothing shows what `timeout` moves

Open
  • Is something wrong?
#280 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
48/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Quiet
Tech stack
typescript
Domain
api, networking

Research direction

Start with lib/drivers/ddp.ts, interfaces/index.ts, and RocketChatClient.url. First add the guard spec for a protocol-bearing host to pin current client.url behavior, then trace the existing deadline call sites and resolved Socket configuration. Done means the deadlines are exposed by name through the shared resolved config, the constructor read-back is removed, and the guard spec passes.

Written by the indexing model from the issue text.

Description

ready-for-agent

Where: lib/drivers/ddp.ts (construction, reopenNow, waitForOpen, ping, alive, probe, waitForNotifyUserMediaSubs), interfaces/index.ts.

Problem

Every Deadline in the driver is computed at its call site, from arithmetic spread across ~550 lines:

  • waitForOpen — config.reopen * 2
  • ping's pong wait — config.ping
  • the alive window — config.ping * 2
  • reopenNow — config.timeout
  • probe — 2000, derived from nothing (deliberately, see ADR-0003)
  • waitForNotifyUserMediaSubs — config.timeout, polling every 100

Each is justified by a local comment repeating ADR-0003. Nothing shows them together, so "what does raising timeout actually move?" can only be answered by reading the whole file. That question came up in #232 and #255 and could not be weighed.

ISocketConfig already gives the resolved options one owner, but it is a bag of raw scalars — it moved the defaults into the Socket and left the arithmetic at the call sites. One leftover from that: this.config = { ...options, timeout: this.ddp.config.timeout } in the DDPDriver constructor copies a single field back out of the Socket's resolved config, so driver.config.timeout is resolved while driver.config.ping is undefined, even though driver.pingInterval reads the resolved one.

Fix

One pure function: raw options in, a fully resolved config out.

{ host, useSsl, timeout, reopen, ping, deadlines: { open, pong, alive, reopenNow, probe, mediaSubs, mediaSubsPoll } }

deadlines is the payload; the scalars are its inputs. Call sites read a name. Socket and Driver both hold the same object, so the read-back in the constructor has nothing left to do and goes away.

Watch out: client.url

RocketChatClient.url returns driver.config.host, and the Driver constructor has already stripped the protocol off it. So client.url hands back a protocol-less host under a name that promises a URL — public API that any change to host normalisation moves silently.

Land a guard spec before the resolver: pass host in with a protocol (https://example.com:3000) and assert what client.url resolves to. Pin the current behaviour first, then decide separately whether it is right.

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.