Issue: https://github.com/BlowaterNostr/nostr.ts/issues/16
Title: Breaking Change Request: redesign updateSub's API
Author: Water Blower (npub1dww…pdnf)
# Problem
Consider
```ts
const subID = "xxx"
const stream1 = await pool.newSub(subID, {...});
const stream2 = await pool.updateSub(subID, {...});
```
Currently, `stream1` and `stream2` are the same reference. Updating a subscription won't kill the channel created by the previous subscription.
This is an abstraction invented by `nostr.ts` since nostr protocol does not have the concept of updating an existing subscription. Our implementation close the previous subscription and creates a new subscription with the same name.
This design looks nice and convenient at first, but after several months' practice, I find this API behavior might not be desireable.
1. Hard to debug
In Blowater's production, we encountered dangling promises or starvation of channels. https://github.com/BlowaterNostr/blowater/issues/115
The root cause is still unknown but this API design should be related because Blowater did not have this problem before introducing `updateSub`
2. Unclear responsibility for channel consumers
In a real world situation, `stream1` and `stream2` are likely to be constructed in different places of the code and in different time. Since `stream1` and `stream2` are the same reference. Their consumers effectly are racing to read from the same channel which might not be desired and might cause surprising data loss from individual consumer's perspective.
3. Suprising Data for consumers
Following No.2, the reader of `stream1` might be surprised that it gets data which doesn't match its original filter.
# Proposal
1. Must
We should close the channel of `stream1` when `updateSub` is called, making `stream1` and `stream2` 2 separate references.
2. Maybe
Forbid the reuse of the same subscription id for different subscriptions and remove `updateSub`.
In this case, the caller of this API should do
```js
const stream1 = await pool.newSub("profileStream 1", {...});
const stream2 = await pool.updateSub("profileStream 2", {...});
```
Use a suffix index to indicate that it's from the same feature but for different iterations.
Nostr GitHub Connect /
npub1mc…hhm00
2023-08-29 15:41:38
Author Public Key
npub1mc9ykrmrpwk3k2x23s9yanqa2gws47reqy342xf39d80sf83x97sjhhm00Seen on
wss://relay.nostr.bandPublished at
2023-08-29 15:41:38Event JSON
{
"id": "39f1bae5a7e1ee8f998e4c93ec183ddaf2d831f39241ec73de5c14c41f953ccc",
"pubkey": "de0a4b0f630bad1b28ca8c0a4ecc1d521d0af87901235519312b4ef824f1317d",
"created_at": 1693323698,
"kind": 1,
"tags": [
[
"p",
"6b9da920c4b6ecbf2c12018a7a2d143b4dfdf9878c3beac69e39bb597841cc6e"
],
[
"url",
"https://github.com/BlowaterNostr/nostr.ts/issues/16"
]
],
"content": "Issue: https://github.com/BlowaterNostr/nostr.ts/issues/16\nTitle: Breaking Change Request: redesign updateSub's API\nAuthor: nostr:npub1dww6jgxykmkt7tqjqx985tg58dxlm7v83sa743578xa4j7zpe3hql6pdnf\n# Problem\r\nConsider\r\n```ts\r\nconst subID = \"xxx\"\r\nconst stream1 = await pool.newSub(subID, {...});\r\nconst stream2 = await pool.updateSub(subID, {...});\r\n```\r\nCurrently, `stream1` and `stream2` are the same reference. Updating a subscription won't kill the channel created by the previous subscription.\r\n\r\nThis is an abstraction invented by `nostr.ts` since nostr protocol does not have the concept of updating an existing subscription. Our implementation close the previous subscription and creates a new subscription with the same name.\r\n\r\nThis design looks nice and convenient at first, but after several months' practice, I find this API behavior might not be desireable.\r\n\r\n1. Hard to debug\r\nIn Blowater's production, we encountered dangling promises or starvation of channels. https://github.com/BlowaterNostr/blowater/issues/115\r\nThe root cause is still unknown but this API design should be related because Blowater did not have this problem before introducing `updateSub`\r\n\r\n2. Unclear responsibility for channel consumers\r\nIn a real world situation, `stream1` and `stream2` are likely to be constructed in different places of the code and in different time. Since `stream1` and `stream2` are the same reference. Their consumers effectly are racing to read from the same channel which might not be desired and might cause surprising data loss from individual consumer's perspective.\r\n\r\n3. Suprising Data for consumers\r\nFollowing No.2, the reader of `stream1` might be surprised that it gets data which doesn't match its original filter.\r\n\r\n# Proposal\r\n1. Must\r\nWe should close the channel of `stream1` when `updateSub` is called, making `stream1` and `stream2` 2 separate references. \r\n\r\n2. Maybe\r\nForbid the reuse of the same subscription id for different subscriptions and remove `updateSub`.\r\n\r\nIn this case, the caller of this API should do\r\n```js\r\nconst stream1 = await pool.newSub(\"profileStream 1\", {...});\r\nconst stream2 = await pool.updateSub(\"profileStream 2\", {...});\r\n```\r\nUse a suffix index to indicate that it's from the same feature but for different iterations.",
"sig": "ad49a9accba6e0820e61f2304aa3154ca5eaff2ec9289f1c589707bfc20cf1e380b8f41ce6c2fd88cb73abb35ddaefee5390723ce1369d2d3d749cea490069bc"
}