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

Issue 793392 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 748549
issue 792974



Sign in to add a comment

Reporting / Network Error Logging: Fix cyclical reporting issues

Project Member Reported by juliatut...@chromium.org, Dec 8 2017

Issue description

I need to make sure that NEL won't report on Reporting uploads, since a failed endpoint could cause an infinite loop of reports, and also the uploader currently cancels requests once the headers are received, which generates an ERR_ABORTED report.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 15 2017

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

commit a8a1b79e55044e6329efcdb983ca939c445e7095
Author: Julia Tuttle <juliatuttle@chromium.org>
Date: Fri Dec 15 15:59:59 2017

Network Error Logging: Ignore Reporting API upload requests

Right now, the ReportingUploader cancels requests once the headers are
received, which causes the requests to fail with ERR_ABORTED, which
triggers a NEL report.

This CL breaks that loop by attaching user data to upload requests and
ignoring ERR_ABORTED errors stemming from them.

TBR=mkwst@chromium.org

Bug:  793392 
Change-Id: Id26516533ec0e7531797717526b015c56a742c1b
Reviewed-on: https://chromium-review.googlesource.com/817553
Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524390}
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/network_error_logging/network_error_logging_service.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/network_error_logging/network_error_logging_service_unittest.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_service.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_service.h
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_test_util.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_test_util.h
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_uploader.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/reporting/reporting_uploader.h
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/url_request/network_error_logging_delegate.h
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/url_request/url_request.cc
[modify] https://crrev.com/a8a1b79e55044e6329efcdb983ca939c445e7095/net/url_request/url_request_unittest.cc

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 15 2017

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

commit 7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Fri Dec 15 18:44:26 2017

Revert "Network Error Logging: Ignore Reporting API upload requests"

This reverts commit a8a1b79e55044e6329efcdb983ca939c445e7095.

Reason for revert: Caused MSan failures. (Thanks Findit!) https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20MSan%20Tests/6611

Original change's description:
> Network Error Logging: Ignore Reporting API upload requests
> 
> Right now, the ReportingUploader cancels requests once the headers are
> received, which causes the requests to fail with ERR_ABORTED, which
> triggers a NEL report.
> 
> This CL breaks that loop by attaching user data to upload requests and
> ignoring ERR_ABORTED errors stemming from them.
> 
> TBR=mkwst@chromium.org
> 
> Bug:  793392 
> Change-Id: Id26516533ec0e7531797717526b015c56a742c1b
> Reviewed-on: https://chromium-review.googlesource.com/817553
> Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
> Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524390}

TBR=mgersh@chromium.org,juliatuttle@chromium.org

Change-Id: I8aae7138cb616fb86e038463c80560f9e916f0c9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  793392 
Reviewed-on: https://chromium-review.googlesource.com/830313
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524419}
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/network_error_logging/network_error_logging_service.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/network_error_logging/network_error_logging_service_unittest.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_service.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_service.h
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_test_util.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_test_util.h
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_uploader.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/reporting/reporting_uploader.h
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/url_request/network_error_logging_delegate.h
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/url_request/url_request.cc
[modify] https://crrev.com/7dc15ab3cc9a1e7f8d4394d011c43f6e52c9a375/net/url_request/url_request_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 15 2017

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

commit 80a699904010b720a7065c18e43b5552e5719de5
Author: Mohsen Izadi <mohsen@chromium.org>
Date: Fri Dec 15 18:56:20 2017

Revert "Network Error Logging: Ignore Reporting API upload requests"

This reverts commit a8a1b79e55044e6329efcdb983ca939c445e7095.

Reason for revert: Failures on Linux MSan Tests buildbot:
https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20MSan%20Tests

Original change's description:
> Network Error Logging: Ignore Reporting API upload requests
> 
> Right now, the ReportingUploader cancels requests once the headers are
> received, which causes the requests to fail with ERR_ABORTED, which
> triggers a NEL report.
> 
> This CL breaks that loop by attaching user data to upload requests and
> ignoring ERR_ABORTED errors stemming from them.
> 
> TBR=mkwst@chromium.org
> 
> Bug:  793392 
> Change-Id: Id26516533ec0e7531797717526b015c56a742c1b
> Reviewed-on: https://chromium-review.googlesource.com/817553
> Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
> Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524390}

TBR=mgersh@chromium.org,juliatuttle@chromium.org

Change-Id: I3e86f06bed769481a2ec6bdec109bdd590493ddc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  793392 
Reviewed-on: https://chromium-review.googlesource.com/830353
Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524423}

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 19 2017

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

commit 4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf
Author: Julia Tuttle <juliatuttle@chromium.org>
Date: Tue Dec 19 18:27:38 2017

Reland "Network Error Logging: Ignore Reporting API upload requests"

Fix an uninitialized field in a unittest that msan caught.

This is a reland of a8a1b79e55044e6329efcdb983ca939c445e7095
Original change's description:
> Network Error Logging: Ignore Reporting API upload requests
>
> Right now, the ReportingUploader cancels requests once the headers are
> received, which causes the requests to fail with ERR_ABORTED, which
> triggers a NEL report.
>
> This CL breaks that loop by attaching user data to upload requests and
> ignoring ERR_ABORTED errors stemming from them.
>
> TBR=mkwst@chromium.org
>
> Bug:  793392 
> Change-Id: Id26516533ec0e7531797717526b015c56a742c1b
> Reviewed-on: https://chromium-review.googlesource.com/817553
> Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
> Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524390}

TBR=mkwst@chromium.org

Bug:  793392 
Change-Id: I4aa40afd379fc70ce61dc2ac94aed2c6dc91e9a1
Reviewed-on: https://chromium-review.googlesource.com/830453
Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525089}
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/network_error_logging/network_error_logging_service.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/network_error_logging/network_error_logging_service_unittest.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_service.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_service.h
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_test_util.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_test_util.h
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_uploader.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/reporting/reporting_uploader.h
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/url_request/network_error_logging_delegate.h
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/url_request/url_request.cc
[modify] https://crrev.com/4667c1c87d4746c53e39be01bd6b7bc4dd8e75bf/net/url_request/url_request_unittest.cc

Sign in to add a comment