New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 882113 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

InspectorWebsocket._SendRequest raised WebSocketConnectionClosedException which disrupts the whole benchmark run

Project Member Reported by nednguyen@chromium.org, Sep 8

Issue description

Log:
  WaitForJavaScriptCondition at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py:239
    return self._inspector_backend.WaitForJavaScriptCondition(*args, **kwargs)
  traced_function at /b/swarming/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py:75
    return func(*args, **kwargs)
  WaitForJavaScriptCondition at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:288
    return py_utils.WaitFor(IsJavaScriptExpressionTrue, timeout)
  WaitFor at /b/swarming/w/ir/third_party/catapult/common/py_utils/py_utils/__init__.py:136
    res = condition()
  IsJavaScriptExpressionTrue at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:285
    return self._EvaluateJavaScript(condition, context_id, timeout)
  traced_function at /b/swarming/w/ir/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py:75
    return func(*args, **kwargs)
  Inner at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:37
    return func(inspector_backend, *args, **kwargs)
  _EvaluateJavaScript at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:533
    self._runtime.Crash(context_id, timeout)
  Crash at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py:81
    res = self._inspector_websocket.SyncRequest(request, timeout)
  SyncRequest at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_websocket.py:127
    self._SendRequest(req)
  _SendRequest at /b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_websocket.py:115
    self._socket.send(data)
  send at /b/swarming/w/ir/third_party/catapult/telemetry/third_party/websocket-client/websocket/_core.py:234
    return self.send_frame(frame)
  send_frame at /b/swarming/w/ir/third_party/catapult/telemetry/third_party/websocket-client/websocket/_core.py:259
    l = self._send(data)
  _send at /b/swarming/w/ir/third_party/catapult/telemetry/third_party/websocket-client/websocket/_core.py:423
    return send(self.sock, data)
  send at /b/swarming/w/ir/third_party/catapult/telemetry/third_party/websocket-client/websocket/_socket.py:113
    raise WebSocketConnectionClosedException("socket is already closed.")
WebSocketConnectionClosedException: socket is already closed.

(https://chrome-swarming.appspot.com/task?id=3fcf14f96a859810&refresh=10&show_raw=1)

We should also wrap the error here with inspector_websocket.WebSocketException (similar to  issue 876681 )
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 8

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/c869f29dcef06183f043f26997e9f158f1974a48

commit c869f29dcef06183f043f26997e9f158f1974a48
Author: Nghia Nguyen <nednguyen@google.com>
Date: Sat Sep 08 12:53:02 2018

Wrap all websocket exception within InspectorWebsocket._SendRequest with exception.Error

This is to make sure that Telemetry can recover its test runs with this type
of error. This CL is very similar to https://chromium-review.googlesource.com/1184974

TBR=perezju@chromium.org
Bug:  chromium:882113 
Change-Id: If96ff724c2e0d9fc0909acffcb4798710037f25e
Reviewed-on: https://chromium-review.googlesource.com/1214865
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/c869f29dcef06183f043f26997e9f158f1974a48/telemetry/telemetry/internal/backends/chrome_inspector/inspector_websocket.py

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f4a363fb191d4a0f49438539d55eea6e4d67b2f

commit 8f4a363fb191d4a0f49438539d55eea6e4d67b2f
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Sat Sep 08 14:57:16 2018

Roll src/third_party/catapult ead23c2a266c..c869f29dcef0 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/ead23c2a266c..c869f29dcef0


git log ead23c2a266c..c869f29dcef0 --date=short --no-merges --format='%ad %ae %s'
2018-09-08 nednguyen@google.com Wrap all websocket exception within InspectorWebsocket._SendRequest with exception.Error


Created with:
  gclient setdep -r src/third_party/catapult@c869f29dcef0

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:882113 
TBR=sullivan@chromium.org

Change-Id: I29c31e0abcedc92dfa3514aa6444c4255279512f
Reviewed-on: https://chromium-review.googlesource.com/1215083
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#589781}
[modify] https://crrev.com/8f4a363fb191d4a0f49438539d55eea6e4d67b2f/DEPS

Labels: Merge-Request-70
11043.0.0 became green.
This need to be merged to R70 branch.

Project Member

Comment 4 by sheriffbot@chromium.org, Sep 11

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I don't understand why the merge request?
Labels: -Hotlist-Merge-Review -Merge-Review-70
Owner: nednguyen@chromium.org
Status: Fixed (was: Untriaged)
#5: I am not sure how that happened :-/
I guess I misunderstood how catapult rolled.

I just found
platform_LogoutPerf and cheets_AuthPerf already passed on R71 but still failed on R70.
https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/testDetails?daysBack=30&testName=platform_LogoutPerf
https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/testDetails?daysBack=30&testName=cheets_AuthPerf

Could you help merge the catapult change into R70 as well?

Kuang-Che - I think you want:
https://bugs.chromium.org/p/chromium/issues/detail?id=879353
This covers the cheets failures.

Sign in to add a comment