Skip to content

fix(dispatcher): catch validation errors in outbound sendEvent/sendCreate#41869

Open
triemerge wants to merge 1 commit into
microsoft:mainfrom
triemerge:fix/dispatcher-catch-outbound-validation
Open

fix(dispatcher): catch validation errors in outbound sendEvent/sendCreate#41869
triemerge wants to merge 1 commit into
microsoft:mainfrom
triemerge:fix/dispatcher-catch-outbound-validation

Conversation

@triemerge

Copy link
Copy Markdown
Contributor

Rationale

The inbound dispatch() path catches validator errors, but the outbound sendEvent/sendCreate side doesn't seem to. A validation failure there can go uncaught and take down the driver process.

Followed the same isUnderTest() convention used elsewhere in this file.

Fixes #41865

try {
const validator = findValidator(type, '', 'Initializer');
initializer = validator(initializer, '', this._validatorToWireContext());
} catch (e) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is likely far too broad of a catch-all (i.e. it could mask other legitimate bugs that need fixing)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense, reverted the sendCreate change and also narrowed the remaining sendEvent catch to ValidationError only while at it.

} catch (e) {
if (isUnderTest())
throw e;
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont think we can safely just drop this as the object has been created (i.e. a future operation with the same guid will fail)

@triemerge triemerge Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the dispatcher is already registered at that point so just returning would leave things in a weird state. Would you prefer the sendCreate path handled separately or is it fine to leave that for a follow up?

Edit: for the sendCreate path, one option could be catching in the Dispatcher constructor instead, where we have access to undo the registration. That way there's no ghost object. happy to add that here if it makes sense..

@triemerge
triemerge force-pushed the fix/dispatcher-catch-outbound-validation branch from 30c3805 to 8ef2d7b Compare July 20, 2026 21:11
@triemerge
triemerge requested a review from dcrousso July 20, 2026 21:25
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.

[Bug]: Unhandled ValidationError in DispatcherConnection crashes the whole driver process on malformed CDP data (connect_over_cdp)

2 participants