Skip to content

fix: preserve callback state when wrapping requests - #252

Open
alexandroit wants to merge 1 commit into
restify:masterfrom
alexandroit:fix/stackline-once-callback-state
Open

alexandroit wants to merge 1 commit into
restify:masterfrom
alexandroit:fix/stackline-once-callback-state

Conversation

@alexandroit

Copy link
Copy Markdown

Summary

HttpClient.request wraps caller-provided callbacks with once. The published
once@1.4.0 wrapper copies enumerable callback properties after initializing
its own state, so a callback that already owns called or value can suppress
its first invocation or expose a value before it runs.

This change:

  • keeps Restify's existing require('once') imports unchanged by using an npm
    alias under the historical once dependency key;
  • resolves that key to exact @stackline/once@1.0.0, an ISC compatibility
    continuation with zero runtime dependencies; and
  • adds a regression through the real HttpClient.request path proving a
    decorated callback starts with clean wrapper state, runs exactly once, and
    retains the wrapper's public called state.

No Restify source import or public API changes.

Package: https://www.npmjs.com/package/@stackline/once/v/1.0.0
Source/release: https://github.com/alexandroit/stackline-once/releases/tag/stackline-v1.0.0

Verification

  • clean Yarn 1.22.22 install from the public npm registry; the once key
    resolves to @stackline/once@1.0.0 with zero runtime dependencies;
  • make lint;
  • make codestyle;
  • deterministic non-environmental suite: 230 passing on Node 22.23.0,
    24.19.0, and 26.8.1;
  • focused reserved once state regression: 1 passing on each of Node 22, 24,
    and 26;
  • direct consumer smoke covering initial state, first/second calls, decoration
    preservation, package identity, and the zero-dependency tree.

For comparison, the focused regression fails against the current
once@1.4.0 dependency because wrappedCallback.called is already true, and
passes with this alias.

I also ran the complete local make coverage suite. It reached 232 passing and
6 host-environment failures, all outside the changed code: fixed port 8080 was
already occupied in the socket-close test, and five tests expecting a delayed
ConnectTimeoutError received an immediate generic network error for
10.255.255.1 on this host. The deterministic runs above exclude the fixed-port
and connect-timeout cases; no other test was excluded or failed.

Disclosure

I maintain @stackline/once, so this pull request proposes a package I publish.
The package is a compatibility-focused continuation of the ISC-licensed once
line and is not affiliated with or endorsed by the original once maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant