fix(plugin): async shutdown hook returns forceFlush Promise + drop sdk.shutdown bus race#78
Merged
Merged
Conversation
…k.shutdown bus race rc3 diagnostic identified two compounding bugs: 1. bcode-laminar's server.instance.disposed bus handler awaits sdk.shutdown(), which unregisters the global TracerProvider AND closes the BatchSpanProcessor. In headless bcode run mode this fires before the top-level finally, so by the time the sync shutdown hook runs the provider is gone and the BSP queue is dropped. 2. The top-level finally fell back to trace.getTracerProvider().forceFlush(), but the OTel API's ProxyTracerProvider does not expose forceFlush, so even when the global was still registered the call was a no-op. Diagnostic stderr line: '[bcode] shutdown: no forceFlush on global provider'. Fix: - Change Hooks.shutdown return type to void | Promise<void> so plugins can return a Promise the host awaits. - Have bcode-laminar's shutdown end any open turn spans then return processor.forceFlush() directly. Closure ref to the processor bypasses the broken global lookup entirely. - Remove the sdk.shutdown() / global-unregister call from the server.instance.disposed bus handler. That handler now only ends remaining spans; the sync hook owns drain. - Drop the trace.getTracerProvider().forceFlush() fallback from src/index.ts and the now-unused trace import. Diagnostic stderr lines stay in place; will be removed in a follow-up after V4 staging verifies the turn parent span lands.
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.
rc3 diagnostic captured two compounding bugs.
**1.**bcode-laminar'sserver.instance.disposedbus handler awaitssdk.shutdown(), which unregisters the global TracerProvider AND closes the BatchSpanProcessor. In headlessbcode runmode this fires before the top-levelfinally, so by the time the sync shutdown hook runs, the provider is gone and the BSP queue is dropped. Diagnostic:[bcode-laminar] shutdown invoked: ending 0 turn span(s).2. The top-levelfinallyfell back totrace.getTracerProvider().forceFlush(), but the OTel API'sProxyTracerProviderdoes not exposeforceFlush— so even when the global was still registered the fallback was a no-op. Diagnostic:[bcode] shutdown: no forceFlush on global provider.**Fix.**``-Hooks.shutdownreturn type widened tovoid | Promiseso plugins can return a Promise the host awaits.-bcode-laminar.shutdownnow ends any still-open turn spans then returnsprocessor.forceFlush()directly. The closure ref toprocessorbypasses the broken global lookup entirely.- Dropawait sdk.shutdown()fromserver.instance.disposed(the handler now just ends remaining spans).-src/index.ts: drop thetrace.getTracerProvider().forceFlush()block and the now-unusedtraceimport. Host awaitsPromise.allSettledof hook returns with a 3s race.Diagnosticprocess.stderr.writelines stay so the next cloud verification can confirmforceFlush done in Xms. They will be removed in a follow-up before v0.1.8 proper.Typecheck: 7/7 packages green.Summary by cubic
Fixes dropped OTel spans during shutdown by making plugin shutdown hooks async and removing the race caused by early SDK teardown. The host now awaits plugin flushes with a 3s cap, and
bcode-laminarflushes its own processor reliably.shutdownhook now supportsvoid | Promise<void>; host awaits all hooks with a 3s timeout.bcode-laminarshutdown ends open turn spans, then returnsprocessor.forceFlush()via closure (no global provider lookup).sdk.shutdown()fromserver.instance.disposedto avoid unregistering the provider and closing the BSP prematurely.trace.getTracerProvider().forceFlush()fallback and the unusedtraceimport; the proxy provider doesn’t exposeforceFlush.Written for commit a6e7a87. Summary will update on new commits. Review in cubic