Guard the invalid session flag reset against a removed session - #492
Merged
Conversation
RemoveInvalidSessionFromDatabase clears the runtime session as soon as the session row was deleted, so the following reset of IsInvalidSession threw a NullReferenceException on exactly the successful path. The exception was swallowed by the surrounding catch and logged as a packet processing error, which hid the real reason and skipped the remaining work in ProcessPacket. A regression test drives a participants packet of an online session through the packet processor with database usage to cover the removal path.
The null-conditional reset after the removal added a branch that cannot be reached by a test, because Remove also reports success when no row matched, so the session is always cleared on that path. SonarCloud therefore failed the quality gate with 66.7% coverage on new code. Resetting the flag before the removal keeps the original intent for a failed removal without adding an untestable condition.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
ProcessPacketresetSession.IsInvalidSessiondirectly after callingRemoveInvalidSessionFromDatabase, which setsSessiontonullas soon as the session row was deleted. On the successful removal path this threw aNullReferenceException. The flag is now reset before the removal, so removing an invalid session no longer produces an error.Linked issues
Closes #491
Review notes
catch, logged as"Error processing packet {PacketType}!"and written toLastError, which hid the real reason and skipped the remaining work inside theif (processor != null)block.RepositoryBase.Removealso reports success when no row matched, so the session is cleared on every reachable path and a guarded reset would add a condition that no test can cover. The behaviour for a failed removal is unchanged, the flag is cleared either way.PacketProcessorInvalidSessionTestsdrives a session start event, a session packet and a participants packet with a team id of 41 through a packet processor withUseDatabase = true. It rewrites the unique session id of the sample packets so the test works on its own database session and does not interfere with the shared test data. The test fails with aNullReferenceExceptioninLastErrorwithout the fix.