Skip to content

[dwds] Fix ping event serialization by introducing PingRequest#2821

Merged
auto-submit[bot] merged 3 commits into
dart-lang:mainfrom
jyameo:jy-fix-ping
Apr 17, 2026
Merged

[dwds] Fix ping event serialization by introducing PingRequest#2821
auto-submit[bot] merged 3 commits into
dart-lang:mainfrom
jyameo:jy-fix-ping

Conversation

@jyameo
Copy link
Copy Markdown
Contributor

@jyameo jyameo commented Apr 17, 2026 •
edited
Loading

Changes:

  • Replaced the raw map {'type': 'ping'} used for client connection checks with a proper PingRequest class. This fixes a TypeError in the browser client when trying to deserialize the raw map as a list (due to expected wire format mismatch), and aligns the ping event with the ongoing migration to standard JSON serialization in DWDS.

Tests:

  • Added unit tests to verify the change.

Related to #2806

@jyameo jyameo mentioned this pull request Apr 17, 2026
Closed
@jyameo jyameo requested review from bkonyi and nshahan April 17, 2026 18:34
mdebbar
mdebbar reviewed Apr 17, 2026
View reviewed changes
Comment thread dwds/lib/src/handlers/dev_handler.dart
IsolateExit() => ['IsolateExit', request.toJson()],
ErrorResponse() => ['ErrorResponse', request.toJson()],
RegisterEvent() => ['RegisterEvent', request.toJson()],
PingRequest() => ['PingRequest', request.toJson()],
Copy link
Copy Markdown

@mdebbar mdebbar Apr 17, 2026

Choose a reason for hiding this comment

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

Do we also need a PingRequest case in _deserializeMessage?

Copy link
Copy Markdown
Contributor Author

@jyameo jyameo Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No, we don't need it because PingRequest is only sent from the server to the client to verify if the client is still alive. Deserialization is only needed in client.dart on the browser side to receive the event.

@jyameo jyameo requested a review from mdebbar April 17, 2026 18:44
nshahan
nshahan approved these changes Apr 17, 2026
View reviewed changes
Comment thread dwds/lib/src/handlers/dev_handler.dart Outdated
ErrorResponse() => ['ErrorResponse', request.toJson()],
RegisterEvent() => ['RegisterEvent', request.toJson()],
PingRequest() => ['PingRequest', request.toJson()],
Map() => request, // Already a raw message (e.g., ping)
Copy link
Copy Markdown
Contributor

@nshahan nshahan Apr 17, 2026

Choose a reason for hiding this comment

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

Can we remove this case now, or at least the comment about ping?

Copy link
Copy Markdown
Contributor Author

@jyameo jyameo Apr 17, 2026

Choose a reason for hiding this comment

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

Removed the comment since it's no longer relevant. I kept the Map case just to be safe for now.

Comment thread dwds/web/client.dart
'ServiceExtensionRequest' => ServiceExtensionRequest.fromJson(jsonData),
'BuildResult' => BuildResult.fromJson(jsonData),
'ErrorResponse' => ErrorResponse.fromJson(jsonData),
'PingRequest' => PingRequest.fromJson(jsonData),
Copy link
Copy Markdown
Contributor

@nshahan nshahan Apr 17, 2026

Choose a reason for hiding this comment

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

IIUC, we deserialize the PingRequest on the injected client side but nothing is done with it. Just sending the message without a failure is enough to tell the server side that the client is still alive?

Copy link
Copy Markdown
Contributor Author

@jyameo jyameo Apr 17, 2026

Choose a reason for hiding this comment

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

yes, that is correct.

@auto-submit auto-submit Bot merged commit e43744b into dart-lang:main Apr 17, 2026
52 checks passed
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request May 1, 2026
@natebosch
[deps] rev ai, core, dartdoc, ecosystem, http, i18n, shelf, test, web…
836d27f 
…, webdev, webdriver

Revisions updated by `dart tools/rev_sdk_deps.dart`.

ai (https://github.com/dart-lang/ai/compare/3d3b7fd..7270c43):
  7270c43  Wed Apr 29 01:14:51 2026 +0530  Sourav-Sonkar  fix: graceful shutdown of orphaned test apps on windows (dart-lang/ai#438)
  07af327  Tue Apr 28 11:11:31 2026 -0700  Jacob MacDonald  Update dependencies (dart-lang/ai#453)
  ff1866d  Wed Apr 29 00:13:33 2026 +0800  Goweii  fix: Add empty properties object to tool input schemas to resolve Claude 400 validation error (dart-lang/ai#444)
  8ce7450  Thu Apr 16 11:03:55 2026 -0700  Jacob MacDonald  add gemini code assist configuration for reviews (dart-lang/ai#443)
  899e562  Tue Apr 14 11:38:29 2026 -0700  Jacob MacDonald  Merge cherry-pick branch into main (dart-lang/ai#442)
  5eae0ae  Tue Apr 14 18:27:49 2026 +0000  Jake Macdonald  Merge remote-tracking branch 'origin/cherry-pick' into merge-cherry-pick
  1e69aa8  Tue Apr 14 11:20:43 2026 -0700  Jacob MacDonald  re-implement most of `#440` for the current stable release (dart-lang/ai#441)

core (https://github.com/dart-lang/core/compare/347df4b..be0b1531):
  be0b1531  Wed Apr 29 16:50:55 2026 -0700  Nate Bosch Revert "Completely new cross-platform API introduced" (#957)
  a23519da  Mon Apr 20 15:57:58 2026 -0700  Nate Bosch  Support variable duration in RestartableTimer (dart-lang/core#949)
  0def20c4  Thu Apr 2 08:40:37 2026 +0200  Lasse R.H. Nielsen  Completely new cross-platform API introduced

dartdoc (https://github.com/dart-lang/dartdoc/compare/c01cf53..2e30b8e):
  2e30b8e3  Tue Apr 21 15:09:21 2026 -0700  Kevin Moore  Improve @canonicalFor validation with close-match suggestions (dart-lang/dartdoc#4234)
  fbc326ea  Tue Apr 14 09:18:26 2026 -0700  Kevin Moore  Fix dartdoc canonicalization for explicit getters and prioritize @canonicalFor (dart-lang/dartdoc#4233)

ecosystem (https://github.com/dart-lang/ecosystem/compare/3bd43d6..ed4e053):
  ed4e053  Wed Apr 15 08:30:59 2026 -0700  Kevin Moore  fix(workflows): prevent duplicate PR comments in post_summaries.yaml (dart-lang/ecosystem#412)

http (https://github.com/dart-lang/http/compare/fa2d2c2..2c84f24):
  2c84f24  Mon Apr 27 14:45:17 2026 -0700  Brian Quinlan  doc: s/dart:html/package:web/g (dart-lang/http#1911)

i18n (https://github.com/dart-lang/i18n/compare/de7e11b..2ae32fd):
  2ae32fdd  Wed Apr 22 02:20:51 2026 -0700  Googler  No public description
  1eaf8a2e  Sun Mar 1 06:51:04 2026 +0000  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/i18n#1053)

shelf (https://github.com/dart-lang/shelf/compare/c4b94d3..cc6b57d):
  cc6b57d  Wed Apr 29 08:59:40 2026 -0700  Kevin Moore  chore: update shelf_io test to not crash on certificate error (dart-lang/shelf#522)
  da9f1b1  Thu Apr 23 14:43:00 2026 -0400  Mouad Debbar  Fix multi-header handling in shelf_proxy (dart-lang/shelf#519)
  a6693a9  Wed Apr 22 23:15:09 2026 -0700  Kevin Moore  feat: Add HTTP/1.1 compliance testing package and baseline for shelf (dart-lang/shelf#518)

test (https://github.com/dart-lang/test/compare/8bbb847..d5da922):
  d5da9229  Mon Apr 27 17:32:33 2026 -0700  Jacob MacDonald  Expose the backend APIs used by flutter_test from test_api (dart-lang/test#2635)
  c8c76f45  Mon Apr 27 13:38:25 2026 -0700  Nate Bosch  Prepare to publish (dart-lang/test#2634)

web (https://github.com/dart-lang/web/compare/15599ee..7c908b1):
  7c908b1  Tue Apr 28 17:16:57 2026 -0700  Kevin Moore  test(js_interop_gen): add integration tests (dart-lang/web#541)
  5ed9bed  Tue Apr 28 17:04:41 2026 -0700  Kevin Moore  js_interop_gen: minimize the number of `@docImport` comments we create (dart-lang/web#540)
  2c47e36  Mon Apr 27 20:55:43 2026 -0700  Kevin Moore  feat: generate descriptive documentation for union typedefs (dart-lang/web#534)
  db91caf  Mon Apr 27 15:15:30 2026 -0700  Kevin Moore  chore: write BCD version to README in web_generator (dart-lang/web#537)
  5e7285f  Fri Apr 24 12:09:10 2026 -0700  Kevin Moore  chore: update node invocation to use the source maps we already generate (dart-lang/web#538)
  3a7e37a  Sat Apr 18 19:31:20 2026 -0700  Kevin Moore  Stabilize JS Interop CI and decouple webref dependencies (dart-lang/web#532)
  d7daf7d  Tue Apr 14 12:23:21 2026 -0700  Kevin Moore  generator: add some sanity to the language version logic (dart-lang/web#530)
  b026317  Mon Apr 13 13:01:37 2026 -0700  Kevin Moore  generator: be more robust against NPM timing issues (dart-lang/web#528)

webdev (https://github.com/dart-lang/webdev/compare/2947c78..d33d270):
  d33d2704  Thu Apr 23 16:00:22 2026 -0400  Jessy Yameogo  [dwds] Sync state with sdk/g3 and restore static analysis for fixtures (dart-lang/webdev#2824)
  e43744b2  Fri Apr 17 16:55:24 2026 -0400  Jessy Yameogo  [dwds] Fix ping event serialization by introducing PingRequest (dart-lang/webdev#2821)
  1063eb00  Fri Apr 17 10:28:47 2026 -0700  MarkZ  Creating experimental flag for web hot reload in webdev and splitting tests between DDC module systems (dart-lang/webdev#2807)

webdriver (https://github.com/google/webdriver.dart/compare/26326d3..3a711eb):
  3a711eb  Mon Apr 27 10:09:31 2026 -0700  Kevin Moore  Update CI badge to point to master branch (google/webdriver.dart#343)

R=bquinlan@google.com

Change-Id: Ie746dfd8f9637db46075b252d3290925de11b382
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499663
Reviewed-by: Brian Quinlan <bquinlan@google.com>
Commit-Queue: Nate Bosch <nbosch@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nshahan nshahan nshahan approved these changes

@bkonyi bkonyi bkonyi approved these changes

@mdebbar mdebbar Awaiting requested review from mdebbar

Assignees

No one assigned

Labels

autosubmit package:dwds

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jyameo @mdebbar @nshahan @bkonyi