Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Aug 25, 2025

This PR addresses the issue where exceptions thrown in service Parse methods were being swallowed due to missing exception handling in the call chain.

Problem

The exception handling flow had a critical gap:

  1. Service implementations (like OidbService, FetchStrangerService) throw exceptions such as OperationException in their Parse methods
  2. These exceptions propagate through EventContext.HandleServerPacketClientListener.ReceiveLoopSocketContext.OnSocketError
  3. The OnSocketError method in SocketContext was empty, causing all exceptions to be silently swallowed
  4. This made debugging service failures extremely difficult as no error information was logged

Solution

BaseService.cs: Added exception handling wrapper around the IService.Parse implementation:

async ValueTask<ProtocolEvent> IService.Parse(ReadOnlyMemory<byte> input, BotContext context)
{
    try
    {
        return await Parse(input, context);
    }
    catch (Exception e)
    {
        context.LogError(Tag, "Parse method failed for service {0}", e, GetType().Name);
        throw;
    }
}

SocketContext.cs: Added meaningful error logging to OnSocketError:

public void OnSocketError(Exception e, ReadOnlyMemory<byte> data)
{
    _context.LogError(Tag, "Socket error occurred during packet processing: {0}", e, e.Message);
}

Key Benefits

  • Preserves existing behavior: All exceptions are still propagated exactly as before
  • Adds visibility: Service parse failures are now logged with specific service names
  • Maintains compatibility: Existing ServiceNotFoundException handling in EventContext remains unchanged
  • Minimal footprint: Only 15 lines of code changed across 2 files

Testing

Verified with mock services that:

  • OperationException from services is properly logged and re-thrown
  • General exceptions from services are properly logged and re-thrown
  • Socket error scenarios are now logged instead of being silently ignored
  • Existing ServiceNotFoundException flow continues to work correctly

Fixes #44.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Author

Copilot AI commented Aug 25, 2025

@DarkRRb 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

…OnSocketError

Co-authored-by: DarkRRb <177549718+DarkRRb@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] [Core] The Parse method of BaseService does not handle exceptions. [Core] Add exception handling to BaseService Parse method and SocketContext OnSocketError Aug 25, 2025
Copilot finished work on behalf of DarkRRb August 25, 2025 04:35
@Copilot Copilot AI requested a review from DarkRRb August 25, 2025 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] The Parse method of BaseService does not handle exceptions.
2 participants