feat(go): harden ergonomic wrapper and unbreak just go check#1784
Merged
Conversation
The two-tier Go binding (raw moq-go-ffi + ergonomic moq-go wrapper) was already in place, but `just go check` was red and several rough edges remained. This hardens the wrapper and fixes two pre-existing breakages that masked each other (the stale test failed vet before linking was ever reached, hiding the missing framework). Pre-existing build fixes: - Update moq_test.go to the current PublishTrack/Consume/SubscribeMedia signatures (they gained info/subscription/maxLatencyMs params). - Add `-framework CoreServices` to the darwin cgo LDFLAGS. libmoq_ffi.a pulls in the notify crate's FSEvents backend, so any binary link (i.e. `go test`) failed with undefined _FSEventStream* symbols. Wrapper hardening: - Re-export all 19 FFI error variants as moq.Err* sentinels plus IsAuthError, so consumers errors.Is without importing moq-go-ffi. - Wrap Session as a real type with Closed(ctx)/Shutdown()/Cancel(code) instead of a bare FFI alias; Serve uses it. - Make Client.Close and Server.Close idempotent via sync.Once. - Promote Transport to a defined type with TransportQUIC/Iroh/WebSocket. - Add Container constructors (LegacyContainer/CmafContainer/LocContainer). - Catalog() now reports ErrClosed on an empty stream instead of (nil, nil). - Document cancellation semantics at the package level and in the README. Testing: - check.sh now runs `go test -race`. - Add concurrency tests covering the runCancellable concurrent-cancel path and repeated/concurrent Cancel; both pass under the race detector. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review follow-ups: - Client.Close/Server.Close: guard against a nil inner so calling Close on a zero-value Client/Server is a no-op (as it was before the sync.Once refactor) rather than panicking in the FFI layer. - Rename ExampleClient_announced to ExampleClient_Announced so the example renders under the Announced method it demonstrates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The two-tier Go binding was already in place (raw
moq-go-ffi+ ergonomicmoq-gowrapper), butjust go checkwas red and the wrapper had a few rough edges. This unbreaks the build and hardens the wrapper. No wire/protocol ormoq-ffichanges; this is wrapper + tooling only. Targetsdevbecause the wrapper API changes are breaking (SessionandTransportwere bare FFI aliases).Pre-existing build fixes (the check was already failing on
dev)These two masked each other: the stale test failed
go vetbefore linking was ever reached, so the missing framework never surfaced.PublishTrack/Consume/SubscribeMediasignatures (they gainedinfo/subscription/maxLatencyMsparams in an earlier refactor).CoreServicesframework (cgo.go):libmoq_ffi.apulls in thenotifycrate's FSEvents backend, so any binary link (i.e.go test) failed with undefined_FSEventStream*symbols on darwin. Added-framework CoreServices.Wrapper hardening
moq.Err*sentinels, plusIsAuthError, so consumerserrors.Iswithout importingmoq-go-ffi.Sessionwrapped as a real type withClosed(ctx)/Shutdown()/Cancel(code)instead of a bare FFI alias;Serveuses it rather than reaching into the raw type.CloseonClientandServerviasync.Once(Serve callsServer.Closeon shutdown).Transportpromoted to a defined type withTransportQUIC/TransportIroh/TransportWebSocket.Containerconstructors:LegacyContainer(),CmafContainer(init),LocContainer().Catalog()reportsErrClosedon an empty stream instead of(nil, nil).Used/Unused/Acceptcalls that have no native cancel).Testing
check.shnow runsgo test -race.runCancellableconcurrent-cancel-during-pending-call path and repeated/concurrentCancel. Both pass under the race detector, validating the core async-to-sync bridge.Test plan
just go checkpasses (go vet,go build,go test -race)-raceand pass (TestRecvGroupCancelRace,TestConsumerCancelConcurrent)gofmt -lcleanPublic API changes (wrapper)
Breaking:
Sessionis now a struct, not a type alias forffi.MoqSession; its methods areClosed(ctx),Shutdown(),Cancel(code uint32).Transportis now a defined type (type Transport string) rather than an alias forstring.Additive:
moq.Err*error sentinels (19),IsAuthError.TransportQUIC/TransportIroh/TransportWebSocketconstants.LegacyContainer/CmafContainer/LocContainerconstructors.(Written by Claude)