Skip to content

Conversation

mrkbac
Copy link
Contributor

@mrkbac mrkbac commented Sep 22, 2025

This fixes:

    raise McapError(f'no channel record found with id {record.channel_id}')
mcap.exceptions.McapError: no channel record found with id 3

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where the McapError "no channel record found with id X" was being raised incorrectly. The fix moves the channel existence check to occur after filtering logic and allows pre-existing schemas and channels to be passed into the _read_inner function.

  • Moved channel existence validation to occur after message filtering logic
  • Added optional schemas and channels parameters to _read_inner function
  • Updated the function call to pass summary schemas and channels when available

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if isinstance(record, Message):
if record.channel_id not in _channels:
raise McapError(f'no channel record found with id {record.channel_id}')
channel = _channels[record.channel_id]
Copy link
Preview

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line will raise a KeyError if the channel_id is not in _channels, but the validation check that prevents this error is now placed after this line (at line 140-141). The validation should occur before attempting to access the channel.

Copilot uses AI. Check for mistakes.

Comment on lines 140 to 141
if record.channel_id not in _channels:
raise McapError(f'no channel record found with id {record.channel_id}')
Copy link
Preview

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation check is now unreachable because line 133 will already raise a KeyError if the channel_id doesn't exist in _channels. The check should be moved before line 133 to be effective.

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@siliconlad siliconlad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for this? Otherwise LGTM!

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.

2 participants