Skip to content

Refactor#349

Merged
0pcom merged 15 commits intoskycoin:developfrom
0pcom:eliminate-internal-packages
Mar 23, 2026
Merged

Refactor#349
0pcom merged 15 commits intoskycoin:developfrom
0pcom:eliminate-internal-packages

Conversation

@0pcom
Copy link
Collaborator

@0pcom 0pcom commented Mar 22, 2026

No description provided.

0pcom added 7 commits March 22, 2026 18:20
… and update dependency graph

Improve test coverage across core packages:
- pkg/disc: 24% -> 85.6% (client lifecycle, HTTP client, entry validation)
- pkg/dmsghttp: 23.8% -> 65.5% (transport, GetServers, ListenAndServe)
- pkg/dmsgctrl: 49.3% -> 84.9% (ServeListener, ping/pong, concurrency)
- pkg/dmsgcurl: 16.2% -> 44.2% (URL parsing, progress writer, CancellableCopy)
- pkg/dmsgpty: 43.1% -> 47.5% (whitelist, RPC utils, config)
- pkg/dmsgserver: 0% -> 88.2% (config generation, flush)

Also update README to use `go run github.com/loov/goda@latest` and
regenerate the dependency graph SVG.
Move all internal packages to pkg/ so they can be imported and tested
by external packages, addressing the testing infrastructure limitation.

Package moves:
- internal/servermetrics -> pkg/dmsg/metrics
- internal/discmetrics -> pkg/disc/metrics
- internal/cli + internal/flags -> pkg/dmsgclient (merged)
- internal/dmsg-discovery/api -> pkg/discovery/api
- internal/dmsg-discovery/store -> pkg/discovery/store
- internal/dmsg-server/api -> pkg/dmsgserver (merged with existing config)
- internal/fsutil -> deleted (inlined os.Stat at single call site)

Only internal/e2e/ remains, containing integration test infrastructure
that is legitimately test-only.

API renames in pkg/dmsgserver: API -> ServerAPI, New -> NewServerAPI
…dd CloseQuietly

Command boilerplate:
- Add ExecName() and Execute() helpers to pkg/dmsgclient
- Replace duplicated Use: expression and Execute() in all 13 cmd packages

Embedded HTML:
- Convert pkg/dmsgpty/ui_html.go from 5738-line hex literal to //go:embed
  with term.html.gz asset file (same runtime behavior)

Panic fixes (library code only, tests left as-is):
- pkg/dmsg/types.go: SignBytes, MakeSignedStreamRequest/Response now return errors
- pkg/dmsg/util.go: encodeGob now returns error
- pkg/dmsg/const.go: shuffleServers now returns error
- pkg/dmsg/metrics/victoria_metrics.go: invalid delta logs instead of panicking
- pkg/dmsgcurl/dmsgcurl.go: String() returns error string instead of panicking
- pkg/dmsgpty/ui.go: writeHeader returns error instead of panicking
- All callers updated to handle new error returns

Error suppression:
- Add pkg/ioutil.CloseQuietly for deferred Close() calls

Also regenerate dependency graph SVG.
File splits (same package, no API changes):
- pkg/dmsg/client.go (723 lines) -> client.go + client_sessions.go + client_dial.go
- cmd/dmsg-discovery/commands/dmsg-discovery.go (533 lines) -> dmsg-discovery.go + examples.go
- pkg/dmsgclient/cli.go (553 lines) -> cli.go + cli_fallback.go

Interface segregation (backwards-compatible, existing interfaces unchanged):
- pkg/disc: Add EntryReader and EntryWriter sub-interfaces; APIClient now embeds them
- pkg/discovery/store: Add EntryStore, ServerLister, EntryEnumerator sub-interfaces;
  Storer now embeds them

All existing implementations continue to satisfy the original interfaces.
The new sub-interfaces allow callers to accept narrower types.
- Run gofmt on cmd files with formatting issues
- Add //nolint:errcheck to test cleanup Close() calls
- Fix indentation in test files
HIGH:
- cmd/dmsgweb: Fix nil deref crash when url.Parse fails in reverse proxy
- cmd/dmsgweb: Add missing wg.Done() in SOCKS5 goroutine (deadlock on shutdown)
- cmd/dmsgweb: Use defer for wg.Done() in proxyHTTPConn (deadlock if panic)
- pkg/dmsg/client: Make errCh send non-blocking to prevent goroutine hang

MEDIUM:
- pkg/dmsg/server: Server.Close() now returns actual error instead of nil
- pkg/dmsg/server: Close conn when smux/yamux Server init fails (TCP leak)
- pkg/dmsg/client_sessions: Close conn on makeClientSession/mux failure (TCP leak)
- pkg/dmsg/client: Retry initial post on failure instead of giving up
- pkg/dmsg/entity_common: Copy session keys while holding mutex (was empty)
- pkg/dmsg/entity_common: Wrap error context in getServerEntry/getClientEntry
- pkg/dmsg/listener: Close drained streams on listener shutdown (resource leak)
- pkg/dmsgserver: Acquire mutex in SetDmsgServer (data race)
- pkg/dmsgcurl: Use caller's context for dmsgC.Serve (cancellation propagation)
- cmd/dmsgweb: Fix duplicate DmsgDiscURL check (second should be DmsgDiscAddr)
- cmd/dmsgweb: Close both conns after io.Copy to unblock goroutine

LOW:
- pkg/dmsg/client: Fix typo "successed" -> "succeeded", stop ticker leak
- pkg/dmsgpty: Use %w instead of %v for error wrapping
- Run gofmt on dmsg-server commands and dmsgserver api
- Add //nolint:errcheck,gosec to test helper functions
- Fix "cancelled" -> "canceled" misspelling (3 test files)
- Add //nolint:gosec for G304 (file variable in test) and G114 (test http.Serve)
@0pcom 0pcom changed the title Eliminate internal packages Refactor Mar 23, 2026
0pcom added 8 commits March 22, 2026 20:34
- github.com/skycoin/skycoin v0.28.3 -> v0.28.5-alpha1 (90b668188f85)
- github.com/skycoin/skywire v1.3.35 -> v1.3.37
- golang.org/x/crypto v0.48.0 -> v0.49.0
- golang.org/x/net v0.51.0 -> v0.52.0
- golang.org/x/sys v0.41.0 -> v0.42.0
- Various other minor updates (smux, VictoriaMetrics, etc.)
- Bump go-version from 1.25.x to 1.26.x in CI workflow
- Bump golangci-lint from v2.6.1 to v2.11.4 (built with go1.26)
- Simplify Makefile lint target to single ./... pass
@0pcom 0pcom merged commit 964219f into skycoin:develop Mar 23, 2026
3 checks passed
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