Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apollo-engine-reporting: fix maxAttempts parameter, log 5xx body #3218

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

glasser
Copy link
Member

@glasser glasser commented Aug 27, 2019

  • The behavior of maxAttempts previously failed to match its documentation or
    the literal reading of its name. Previously, setting maxAttempts=1 would
    actually result in one retry after the initial attempt. This change fixes
    the behavior to match the docs and the name.

  • We intend the bodies of Engine report endpoint errors to be useful in error
    logs, even 5xx errors. This change returns to including them in the reported
    error.

PR #1274 (merged before the initial release of apollo-engine-reporting)
regressed both of these issues.

- The behavior of maxAttempts previously failed to match its documentation or
  the literal reading of its name. Previously, setting maxAttempts=1 would
  actually result in one retry after the initial attempt. This change fixes
  the behavior to match the docs and the name.

- We intend the bodies of Engine report endpoint errors to be useful in error
  logs, even 5xx errors. This change returns to including them in the reported
  error.

PR #1274 (merged before the initial release of apollo-engine-reporting)
regressed both of these issues.
@glasser glasser force-pushed the glasser/reporting-include-error-body branch from 781108d to 455e57f Compare August 27, 2019 18:28
@glasser glasser changed the title apollo-engine-reporting: log 5xx response body apollo-engine-reporting: fix maxAttempts parameter, log 5xx body Aug 27, 2019
@trevor-scheer trevor-scheer merged commit 4675c67 into master Aug 27, 2019
@trevor-scheer trevor-scheer deleted the glasser/reporting-include-error-body branch August 27, 2019 18:40
@glasser
Copy link
Member Author

glasser commented Aug 27, 2019

Just a note @abernix that this got more stuff in it after you approved.

@abernix
Copy link
Member

abernix commented Aug 28, 2019

I appreciate the post-factum ping, @glasser!

@abernix abernix added this to the Release 2.9.1 milestone Aug 29, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants