{{ message }}
Repository navigation
fix(types): add streamDiagEvent to TailStream.TailEventHandlerObject - #7848
Open
PedroRosalba wants to merge 1 commit into
Open
PedroRosalba wants to merge 1 commit into
PedroRosalba wants to merge 1 commit into
Conversation
The object form of a tailStream() handler had no key for StreamDiagnostic
events, although StreamDiagnostic is a member of TailStream.EventType. The
runtime dispatches those events to the handler-object key "streamDiagEvent"
(getHandlerName in src/workerd/io/trace-stream.c++), so in typed code
return { streamDiagEvent(ev) { ... } }
failed to compile (TS2353: 'streamDiagEvent' does not exist in type
'TailEventHandlerType') while workerd calls exactly that key with an event
whose type is "streamDiagnostic" and whose diagnostic is
{ diagnosticsType: "droppedEvents", count }. A typed object handler could
not observe dropped tail events.
23c0d98 added the StreamDiagnostic interface, the EventType member and the
C++ dispatch key together but never added the handler-object key. Declare
streamDiagEvent?: TailEventHandler<StreamDiagnostic> in
types/defines/trace.d.ts and the generated snapshots. The key name follows
the runtime; it is not renamed here because that would change behaviour for
existing handlers.
Test: types/test/types/tail-stream-diag.ts (bazel test
//types:test/types/tail-stream-diag) asserts the key's handler type and that
the handler parameter is TailEvent<StreamDiagnostic>; it fails to compile
without the define change.
|
This pull request was not reviewed automatically because its author does not have write access to this repository. A maintainer with write access can start a review by commenting |
Author
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.

Problem
TailStream.TailEventHandlerObject, the object form atailStream()handler may return, has no key forStreamDiagnosticevents, althoughStreamDiagnosticis a member ofTailStream.EventType. The runtime dispatches those events to the handler-object keystreamDiagEvent(getHandlerNameinsrc/workerd/io/trace-stream.c++), so in typed codefails with TS2353 (
'streamDiagEvent' does not exist in type 'TailEventHandlerType') while workerd calls exactly that key with an event of type"streamDiagnostic"whose diagnostic is{ diagnosticsType: "droppedEvents", count }. A typed object handler cannot observe dropped tail events. The function form of the handler was already correct through theEventTypedefault.Origin: 23c0d98 (#5637) added the
StreamDiagnosticinterface, theEventTypemember and the C++ dispatch key together. In that PR's review theEventsuffix was dropped from the TS interface and thetypestring (StreamDiagnosticsEvent→StreamDiagnostic), but the C++ handler key kept it and the handler-object key was never added.Fix
Declare
streamDiagEvent?: TailEventHandler<StreamDiagnostic>intypes/defines/trace.d.tsand the four snapshot files, with a type test. Additive, types only; the key name follows the shipping runtime.Alternative
streamDiagEventis the one handler key that does not equal its eventtype(every other key does). If you consider the object-form keys not yet public, the cleaner fix is a C++ plus types rename of the key tostreamDiagnostic, as 91f6875 did forattributes. I did not do that here because it would change behaviour for any handler already usingstreamDiagEvent; happy to switch if you prefer.Tests
bazel test //types:test/types/tail-stream-diag(new; asserts the key's handler type and that the handler parameter isTailEvent<StreamDiagnostic>; fails on the previous snapshot)bazel test //src/workerd/api/tests:tail-worker-test@(runtime unchanged)just generate-typesreproduces the snapshot edits byte for byte..wd-testthat the object handler'sstreamDiagEventis called withdiagnosticsType === "droppedEvents"and that astreamDiagnostickey is never called.Related: #7846 touches the same
trace.d.ts(different members); the two apply in either order.Tested on macOS (Apple Silicon) only.