Skip to content

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

Merged
trevor-scheer merged 1 commit intomasterfrom
glasser/reporting-include-error-body
Aug 27, 2019
Merged

apollo-engine-reporting: fix maxAttempts parameter, log 5xx body#3218
trevor-scheer merged 1 commit intomasterfrom
glasser/reporting-include-error-body

Conversation

@glasser
Copy link
Copy Markdown
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
Copy Markdown
Member Author

glasser commented Aug 27, 2019

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

@abernix
Copy link
Copy Markdown
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