Count a GATT error as a single connection attempt failure |
|||||||
Issue descriptionWe retry connection failures up to 3 times if the failure is due to an inability to form a connection to another device. However, if we *do* successfully connect but fail to create a GATT connection, find GATT characteristics, start a notify session, etc., we fail the connection attempt immediately, without any additional retries. The result is that we sometimes treat ephemeral errors as fatal connection issues. We shouldn't do this since it can prevent sending remote messages when we want them to be sent.
,
Aug 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af53bcce33e8b1d568cbd81a26f132fff4ec54d3 commit af53bcce33e8b1d568cbd81a26f132fff4ec54d3 Author: Kyle Horimoto <khorimoto@google.com> Date: Wed Aug 23 18:55:28 2017 [CrOS Tether] Retry MessageTransferOperations for all connection errors. Previously, authentication failures were considered fatal, so any device which failed to authenticate was immediately unregistered. The logic behind this decision was that an authentication error meant that the two devices had out-of-data BeaconSeeds. However, failures during authentication can actually mean several things, such as ephemeral GATT-related errors to timeouts. We should still perform multiple retries in this case. Bug: 758015 , 672263 Change-Id: I09656f19c56b5640ef3ba89585bd24ff154c2a20 Reviewed-on: https://chromium-review.googlesource.com/627858 Commit-Queue: Ryan Hansberry <hansberry@chromium.org> Reviewed-by: Ryan Hansberry <hansberry@chromium.org> Cr-Commit-Position: refs/heads/master@{#496755} [modify] https://crrev.com/af53bcce33e8b1d568cbd81a26f132fff4ec54d3/chromeos/components/tether/message_transfer_operation.cc [modify] https://crrev.com/af53bcce33e8b1d568cbd81a26f132fff4ec54d3/chromeos/components/tether/message_transfer_operation_unittest.cc
,
Aug 23 2017
,
Aug 23 2017
This bug requires manual review: We are only 12 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 23 2017
Approving merge to M61 Chrome OS.
,
Aug 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4bf2274c98480f36631f80b67663448bd0be5319 commit 4bf2274c98480f36631f80b67663448bd0be5319 Author: Kyle Horimoto <khorimoto@google.com> Date: Wed Aug 23 19:31:04 2017 [CrOS Tether] Retry MessageTransferOperations for all connection errors. Previously, authentication failures were considered fatal, so any device which failed to authenticate was immediately unregistered. The logic behind this decision was that an authentication error meant that the two devices had out-of-data BeaconSeeds. However, failures during authentication can actually mean several things, such as ephemeral GATT-related errors to timeouts. We should still perform multiple retries in this case. TBR=khorimoto@google.com (cherry picked from commit af53bcce33e8b1d568cbd81a26f132fff4ec54d3) Bug: 758015 , 672263 Change-Id: I09656f19c56b5640ef3ba89585bd24ff154c2a20 Reviewed-on: https://chromium-review.googlesource.com/627858 Commit-Queue: Ryan Hansberry <hansberry@chromium.org> Reviewed-by: Ryan Hansberry <hansberry@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#496755} Reviewed-on: https://chromium-review.googlesource.com/629657 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#832} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/4bf2274c98480f36631f80b67663448bd0be5319/chromeos/components/tether/message_transfer_operation.cc [modify] https://crrev.com/4bf2274c98480f36631f80b67663448bd0be5319/chromeos/components/tether/message_transfer_operation_unittest.cc
,
Aug 23 2017
,
Jan 22 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by khorimoto@chromium.org
, Aug 23 2017