Fix SessionData counters across multiple errors #583
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
The
SessionData
middleware is intended to keep counters for the number of handled and unhandled errors that occured in the current request. However it was not actually incrementing the counts, so the would always be reported as '1' or '0'This change ensures that we tell the SessionTracker about the updated data, so the counters are correct
Changeset
Essentially the fix is to call
SessionTracker::setCurrentSession
after incrementing the counts, so that it can copy back the updated data. This method didn't used to be public, but it doesn't seem important to keep protectedThere is technically also a small BC break in the
SessionData
constructor parameter has changed fromClient
toSessionTracker
. This is because it only used theClient
to get theSessionTracker
anyway, so it might as well take theSessionTracker
directly. I don't think this will affect many/any users, given this class is constructed internally inClient
and not directly exposed. The change required to fix it is also minimal — passSessionTracker
instead ofClient
, which can be fetched fromClient::getSessionTracker
if necessaryTests
I rewrote the
SessionData
tests to use less mocking — now only theSessionTracker
HttpClient
is mockedI've also added tests to ensure that the counters persist between calls to the middleware — they run the middleware 5 times and assert the counts increment
Review
For the submitter, initial self-review:
For the pull request reviewer(s), this changeset has been reviewed for: