Skip to content

Conversation

@annagrin
Copy link
Contributor

@annagrin annagrin commented Jul 14, 2021

  • Add --log-requests flag to build runner daemon command.
  • Log request failures in asset server.
  • Configure http client in tests to limit max connections per host,
    to prevent CI from failing with reset connection errors.

Extra logging will help diagnose CI flakes in build:

https://github.com/dart-lang/build/runs/3062926301

and webdev:

Related: dart-lang/webdev#1345

@google-cla google-cla bot added the cla: yes Google is happy with the PR contributors label Jul 14, 2021
@annagrin
Copy link
Contributor Author

annagrin commented Jul 20, 2021

@natebosch @grouma @jakemac53 I am thinking about cancelling this PR, as I actually only need one change in it (logging the exception that happens in the asset server handler). I realized that I can log requests from webdev as well, without adding flags to the build daemon, and the rest of the changes are just to enable more logging to debug the failures I encountered in this PR.

Anyway, it helped me narrow down the actual issue and find the solution for it so there was a benefit:) I'll submit a new one just for logging exceptions.

Update: would be still good to keep the tests. I will remove all the parts I don't need and keep http config change, tests, and logging exception in asset server handler.

Update: I decided to just modify this PR instead of cancelling. I kept the --log-requests flag to the daemon as well as the changes mentioned above, it can be useful for debugging the requests to the server.

@annagrin annagrin force-pushed the annagrin/add_daemon_logs branch from 54ac558 to ea520e5 Compare July 21, 2021 23:30
@annagrin
Copy link
Contributor Author

@natebosch @jakemac53 @grouma I addressed (i think) all comments.

Changes I kept - let me know if you would like me to remove any of them:

  • add a --log-requests flag to build daemon
  • test parallel download of many files in _test/test/serve_integration_tests.dart
  • test parallel download of many files in build_runner/test/serve_integration_tests.dart
  • log failures in asset server handler
  • limit simultaneous connections on http client in tests to prevent flakes in CI.

Co-authored-by: Nate Bosch <nbosch1@gmail.com>
@annagrin annagrin merged commit 8526460 into dart-lang:master Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Google is happy with the PR contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants