Read every occurrence of a repeatable TLV, and drop the unread tlvMap #25

Merged
lilleman merged 11 commits from multiple-tlvs into main 2026-09-25 18:54:29 +02:00
4 changed files with 28 additions and 39 deletions
Showing only changes of commit 01210e734a - Show all commits
+8 -20
View File
@@ -12,7 +12,7 @@ not for structure or style.
## Goals ## Goals
The ten goals, in priority order, live in The goals, in priority order, live in
[README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals) — they say where this library is heading, which an outside [README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals) — they say where this library is heading, which an outside
reader judges it by. The README states the audience alongside them. Everything below cites a goal by reader judges it by. The README states the audience alongside them. Everything below cites a goal by
number. number.
@@ -86,8 +86,9 @@ src/
types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays
``` ```
Dependency direction is one way: `defs` knows nothing above it, `pdu` uses `defs`, `session` uses Imports point one way: `defs` knows nothing above it but `result.ts`, `pdu` uses `defs`, `session`
`pdu`, and `client`/`server` use `session`. Nothing reaches back up. uses `pdu`, and `client`/`server` use `session`. The one way back up is the `Session` handed to
`IncomingRequests` and `createSms()`, imported as a type only.
**Parameter order is wire order.** The key order inside `cmds.*.params` is the order the fields are **Parameter order is wire order.** The key order inside `cmds.*.params` is the order the fields are
written to and read from the buffer. Never sort those alphabetically — the alphabetical-ordering written to and read from the buffer. Never sort those alphabetically — the alphabetical-ordering
@@ -103,20 +104,12 @@ docker compose run --rm node npm test
docker compose run --rm node npm run build docker compose run --rm node npm run build
``` ```
- Tests are `.ts` and run directly under Node's type stripping — no build step in the dev loop.
- Source imports use `.ts` extensions; `rewriteRelativeImportExtensions` emits `.js` into `dist`. - Source imports use `.ts` extensions; `rewriteRelativeImportExtensions` emits `.js` into `dist`.
- `erasableSyntaxOnly` is on, so no enums, no namespaces, no parameter properties. Use `as const` - `erasableSyntaxOnly` is on, so no enums, no namespaces, no parameter properties. Use `as const`
objects plus union types. objects plus union types.
- The published floor is Node 18, but the dev container runs Node 24 (type stripping needs it). CI - The published floor is Node 18; the dev container runs Node 24 because type stripping needs it.
compiles the tests and runs them on 18, every LTS above it, and current, so the floor is
verified rather than asserted.
- `typescript` is pinned to the 6.x line because `typescript-eslint` peer-requires `<6.1.0`. Move to - `typescript` is pinned to the 6.x line because `typescript-eslint` peer-requires `<6.1.0`. Move to
TypeScript 7 once that constraint lifts. TypeScript 7 once that constraint lifts.
- GitHub mirrors Gitea through `.gitea/workflows/mirror.yaml`, which never prunes, and
`mirror-delete.yaml`, one run per deleted ref. A delete run that fails or outlives Gitea's queue
timeout, or a push run that cloned before the delete, leaves the ref on GitHub until the delete
run is re-run. Accepted: a stale ref there is harmless, and refs only GitHub has must survive.
Maintainer's call, 2026-09-14; valid while nothing deploys from GitHub.
## Defects found in 0.4.0 ## Defects found in 0.4.0
@@ -152,13 +145,6 @@ Confirmed by reading the 0.4.0 source; each row has a regression test naming the
| `ESME_RINVBCASTCHANIND` typo | Defined as `0x011`, three hex digits; the spec value is `0x0112` | | `ESME_RINVBCASTCHANIND` typo | Defined as `0x011`, three hex digits; the spec value is `0x0112` |
| Every response carries a message id | `session.js` builds `params = {'message_id': …}` for every response it sends, `deliver_sm_resp` included; SMPP 3.4 4.6.2 makes that field unused and NULL, and Jasmin closes the connection on one | | Every response carries a message id | `session.js` builds `params = {'message_id': …}` for every response it sends, `deliver_sm_resp` included; SMPP 3.4 4.6.2 makes that field unused and NULL, and Jasmin closes the connection on one |
## Multipart sends
`sendSms` puts every segment of a message on the wire together instead of waiting for each response
in turn, so a long message costs one round trip rather than one per segment. Nothing on the
receiving side forces the order either way: this library answers each inbound segment as it arrives,
so a peer that dispatches one request at a time is never left waiting on us.
## GSM 7-bit is sent unpacked ## GSM 7-bit is sent unpacked
Over SMPP the ESME puts one GSM character per octet in `short_message` and the SMSC packs it into Over SMPP the ESME puts one GSM character per octet in `short_message` and the SMSC packs it into
@@ -249,7 +235,6 @@ the file.
- `Session` is publicly constructible, which is what makes `SessionOptions` and `ReconnectOptions` - `Session` is publicly constructible, which is what makes `SessionOptions` and `ReconnectOptions`
public too. public too.
- `acceptsOptionalParams()` and `bindAllows()` are predicates, not chokepoints. - `acceptsOptionalParams()` and `bindAllows()` are predicates, not chokepoints.
- `session.sock` is a getter over `PduTransport`.
- Both emitters re-declare their listener methods to accept a promise. - Both emitters re-declare their listener methods to accept a promise.
- `PduRefusedError` is exported, and `sessionError` names it in the event's type. - `PduRefusedError` is exported, and `sessionError` names it in the event's type.
- `bitCount()`, `encodeMessage()` and `splitMessage()` keep their total signatures, because - `bitCount()`, `encodeMessage()` and `splitMessage()` keep their total signatures, because
@@ -308,6 +293,7 @@ the file.
`false` is the one way to turn it off. `false` is the one way to turn it off.
- A stream this library cannot frame is a dead link; one PDU it cannot parse is not. - A stream this library cannot frame is a dead link; one PDU it cannot parse is not.
- A deliberate shutdown drains; an unusable link and an abort do not. - A deliberate shutdown drains; an unusable link and an abort do not.
- `sendSms()` puts every segment of a message on the wire together.
- Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the - Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the
application's own signal rather than the peer's answer. application's own signal rather than the peer's answer.
- `server()` composes the application's `onRequest` after its own bind handling, and offers it every - `server()` composes the application's `onRequest` after its own bind handling, and offers it every
@@ -331,3 +317,5 @@ the file.
- `test/` stays flat too, and a file there is named for the question it answers rather than for the - `test/` stays flat too, and a file there is named for the question it answers rather than for the
module it covers. module it covers.
- CI tests on Linux only; `src/` keeps off what is known to break on macOS or Windows. - CI tests on Linux only; `src/` keeps off what is known to break on macOS or Windows.
- GitHub mirrors Gitea without pruning, and a ref deleted on Gitea is deleted on GitHub by a run of
its own.
+3 -5
View File
@@ -161,7 +161,6 @@ await smpp.close(); // stop listening, then drain and close every live sess
`sendDlr('UNDELIVERABLE')` any other state: [Server in depth](#server-in-depth). `sendDlr('UNDELIVERABLE')` any other state: [Server in depth](#server-in-depth).
- A message that arrived in several segments was answered as they arrived, so `sendResp()` there - A message that arrived in several segments was answered as they arrived, so `sendResp()` there
takes no `smsId` or refusing `status`. `sms.answeredOnArrival` says which case you are in. takes no `smsId` or refusing `status`. `sms.answeredOnArrival` says which case you are in.
- `smpp.close()` stops listening, then drains and closes every live session.
## Errors ## Errors
@@ -262,7 +261,7 @@ All optional. Timeouts are milliseconds.
| `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. | | `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. |
| `idleTimeout` | `40000` | Drop a peer that has been silent this long. | | `idleTimeout` | `40000` | Drop a peer that has been silent this long. |
| `maxReassembly` | `1000` | Incomplete multipart messages held per session. | | `maxReassembly` | `1000` | Incomplete multipart messages held per session. |
| `maxOctets` | `67108864` | Bytes of incomplete multipart messages held per session. | | `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session. |
| `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. | | `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. |
| `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | | | `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | |
@@ -701,7 +700,7 @@ one wins. They do not override the [hard rules](https://gitea.larvit.se/larvit/s
to the application to persist. Coordinating processes any other way is declined without a fresh to the application to persist. Coordinating processes any other way is declined without a fresh
argument each time. argument each time.
10. **It builds, tests and runs the same everywhere.** Container-only toolchain, no runtime 10. **It builds, tests and runs the same everywhere.** Container-only toolchain, no runtime
dependencies, the Node 18 floor verified in CI rather than asserted, every README example executed dependencies, the Node 18 floor verified in CI, every README example executed
by the suite. by the suite.
## Audience ## Audience
@@ -710,8 +709,7 @@ Who depends on this library, and what they may rely on.
- **The public npm audience.** Only what `src/index.ts` exports is public; everything behind it is - **The public npm audience.** Only what `src/index.ts` exports is public; everything behind it is
reshaped freely. reshaped freely.
- **Node 18 and newer, ESM only, no runtime dependencies.** The floor is verified in CI rather than - **Node 18 and newer, ESM only, no runtime dependencies.**
asserted, so the library drops into a service or a container without pulling a tree behind it.
- **Real SMSCs and ESMEs as operators actually run them**, not a reference implementation. Jasmin, - **Real SMSCs and ESMEs as operators actually run them**, not a reference implementation. Jasmin,
SMPPSim, Kannel, jsmpp, Cloudhopper, python-smpplib and php-smpp are the interop targets, and what SMPPSim, Kannel, jsmpp, Cloudhopper, python-smpplib and php-smpp are the interop targets, and what
they do in practice outranks what the specification says they should do. they do in practice outranks what the specification says they should do.
+15 -8
View File
@@ -17,9 +17,6 @@ rule and an index of the titles below.
sending them. Only `submit_sm`, `deliver_sm` and `data_sm` are policed by bind direction — the sending them. Only `submit_sm`, `deliver_sm` and `data_sm` are policed by bind direction — the
three the library dispatches by it, of which it sends the first two. three the library dispatches by it, of which it sends the first two.
- **`session.sock` is a getter over `PduTransport`.** Reading it is unchanged; assigning it no longer
compiles, which never rewired the handlers and so never worked.
- **Both emitters re-declare their listener methods to accept a promise.** Maintainer's call, - **Both emitters re-declare their listener methods to accept a promise.** Maintainer's call,
2026-08-27: `EventEmitter` types every listener as void-returning, so the 2026-08-27: `EventEmitter` types every listener as void-returning, so the
`session.on('sms', async sms => …)` README documents reads as a misused promise in any strict `session.on('sms', async sms => …)` README documents reads as a misused promise in any strict
@@ -133,8 +130,7 @@ rule and an index of the titles below.
rule purely additive: no PDU that parsed before reads differently now. Rejected: preferring the rule purely additive: no PDU that parsed before reads differently now. Rejected: preferring the
TLV, which re-reads every message a peer echoes into both. Rejected: refusing a PDU carrying both, TLV, which re-reads every message a peer echoes into both. Rejected: refusing a PDU carrying both,
which discards a message that is almost certainly present twice over, where goal 3 keeps the which discards a message that is almost certainly present twice over, where goal 3 keeps the
traffic. The reassembler's octet cap already counts TLV values, so a 64 KB payload is bounded like traffic.
any other segment.
- **A segment's concatenation is read from its UDH, or from the `sar_*` TLVs where it declares none, - **A segment's concatenation is read from its UDH, or from the `sar_*` TLVs where it declares none,
and each spelling groups in a reference space of its own.** Maintainer's call, 2026-09-06, from and each spelling groups in a reference space of its own.** Maintainer's call, 2026-09-06, from
@@ -359,8 +355,8 @@ rule and an index of the titles below.
it. There is one budget, 140 less the UDH, and the alphabet decides only what it is counted in, so it. There is one budget, 140 less the UDH, and the alphabet decides only what it is counted in, so
Latin-1 and UCS2 both take those 134 octets — 134 characters and 67 — and it is GSM 7-bit's 153 Latin-1 and UCS2 both take those 134 octets — 134 characters and 67 — and it is GSM 7-bit's 153
that is the odd number rather than the other way round. `Record<EncodingName, number>` is what makes that is the odd number rather than the other way round. `Record<EncodingName, number>` is what makes
a fourth alphabet state its own. Rejected: 134 for GSM 7-bit too, which is the mistake the a fourth alphabet state its own. Rejected: 134 for GSM 7-bit too, which is the mistake
unpacked-alphabet section above exists to stop. Accepted: a Latin-1 message past the 140 characters [GSM 7-bit is sent unpacked](../AGENTS.md#gsm-7-bit-is-sent-unpacked) exists to stop. Accepted: a Latin-1 message past the 140 characters
one SMS holds now costs more segments than it did, and `smsIds` is that much longer. one SMS holds now costs more segments than it did, and `smsIds` is that much longer.
- **An alphabet the caller named has to carry the message, and a time the format cannot express is - **An alphabet the caller named has to carry the message, and a time the format cannot express is
@@ -588,6 +584,10 @@ rule and an index of the titles below.
reports each session's unfinished drain through `serverError`, because its own result says nothing reports each session's unfinished drain through `serverError`, because its own result says nothing
but that the listener stopped. but that the listener stopped.
- **`sendSms()` puts every segment of a message on the wire together.** Goal 6: a long message costs
one round trip rather than one per segment. Rejected: sending each segment once the last is
answered, which a receiver waiting for the whole message before answering would deadlock.
- **Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the - **Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the
application's own signal rather than the peer's answer.** Maintainer's call, 2026-09-06, from the application's own signal rather than the peer's answer.** Maintainer's call, 2026-09-06, from the
Jasmin interoperability phase: Jasmin dispatches one `submit_sm` per connector at a time and will Jasmin interoperability phase: Jasmin dispatches one `submit_sm` per connector at a time and will
@@ -789,7 +789,7 @@ rule and an index of the titles below.
dev image has no openssl. dev image has no openssl.
- **`src/` stays flat until a module has to move for another reason.** Architecture review, - **`src/` stays flat until a module has to move for another reason.** Architecture review,
2026-09-06: the grouping the file map above already implies — `wire/` for `pdu*` and `defs`, 2026-09-06: the grouping the [file map](../AGENTS.md#architecture) already implies — `wire/` for `pdu*` and `defs`,
`link/` for `link-*`, `reconnect-*`, `pdu-transport` and `send-window`, `messages/` for `sms*`, `link/` for `link-*`, `reconnect-*`, `pdu-transport` and `send-window`, `messages/` for `sms*`,
`dlr*`, `message*`, `reassembly` and `udh` — rewrites every import for no change to `dlr*`, `message*`, `reassembly` and `udh` — rewrites every import for no change to
`dist/index.js`, the one published entry. Valid while that map is what a reader navigates by. `dist/index.js`, the one published entry. Valid while that map is what a reader navigates by.
@@ -809,3 +809,10 @@ rule and an index of the titles below.
That binds what `dist/` runs; the container tooling, `interop-tests/` and the `package.json` scripts That binds what `dist/` runs; the container tooling, `interop-tests/` and the `package.json` scripts
run on Linux by goal 10. Rejected: macOS and Windows runners, on GitHub's mirror or as Gitea run on Linux by goal 10. Rejected: macOS and Windows runners, on GitHub's mirror or as Gitea
host-mode runners on a Windows VM and a Mac. host-mode runners on a Windows VM and a Mac.
- **GitHub mirrors Gitea without pruning, and a ref deleted on Gitea is deleted on GitHub by a run of
its own.** Maintainer's call, 2026-09-14; valid while nothing deploys from GitHub.
`.gitea/workflows/mirror.yaml` never prunes, and `mirror-delete.yaml` runs once per deleted ref. A
delete run that fails or outlives Gitea's queue timeout, or a push run that cloned before the
delete, leaves the ref on GitHub until the delete run is re-run. Accepted: a stale ref there is
harmless, and refs only GitHub has must survive.
+2 -6
View File
@@ -262,8 +262,8 @@ and is also what the panel ranked hardest — two methods, one answer.
- [ ] **Give `IncomingRequests` a port instead of the `Session` it drives.** It holds its owner and - [ ] **Give `IncomingRequests` a port instead of the `Session` it drives.** It holds its owner and
calls eight members of it 18 times, including `this.session.close()` on an inbound `unbind` — calls eight members of it 18 times, including `this.session.close()` on an inbound `unbind` —
a collaborator ending its owner's life. `OutgoingRequests` is the mirror half of the same a collaborator ending its owner's life. `OutgoingRequests` is the mirror half of the same
boundary and takes no session at all. AGENTS.md's "Nothing reaches back up" is false because of boundary and takes no session at all. AGENTS.md names this as the one way back up; the port
this, and `docs/decisions.md` already states the rule under The session's life: "a collaborator removes that exception, and `docs/decisions.md` already states the rule under The session's life: "a collaborator
that has to ask does not own its decision". It is also the missing test seam — inbound routing, that has to ask does not own its decision". It is also the missing test seam — inbound routing,
reassembly dispatch, `onRequest` ordering and bind-direction refusal have no unit test because reassembly dispatch, `onRequest` ordering and bind-direction refusal have no unit test because
the class cannot be built without a live socket. Carry the eight members as `IncomingDeps`, the class cannot be built without a live socket. Carry the eight members as `IncomingDeps`,
@@ -399,10 +399,6 @@ and is also what the panel ranked hardest — two methods, one answer.
the fenced blocks at test time and assert each appears verbatim in the executed source, so an the fenced blocks at test time and assert each appears verbatim in the executed source, so an
edit to either fails the gate. edit to either fails the gate.
- [ ] **Correct AGENTS.md's "Nothing reaches back up".** False while `IncomingRequests` holds a
`Session`: either the first Locality item makes it true, or the sentence names the exception
until it does.
- [ ] **Narrow the `src/defs/*` lint exemption to the four table files.** Its stated reason — "the - [ ] **Narrow the `src/defs/*` lint exemption to the four table files.** Its stated reason — "the
spec tables are data: their length tracks the specification, not any complexity" — is false for spec tables are data: their length tracks the specification, not any complexity" — is false for
`defs/types.ts`, which is 595 lines of wire codec with 25 functions and is the file that parses `defs/types.ts`, which is 595 lines of wire codec with 25 functions and is the file that parses