Improved Handling of Unknown EventKind in EventIterator

[msk]

Description

EventIterator currently navigates through events stored in EventDb, deserializing them based on their EventKind. When encountering an EventKind not present in the EventKind enum, EventIterator::next returns Some(Err(InvalidEvent::Key(k))).

This behavior can cause issues when a review agent sends a new type of event that's not represented in the EventKind enum and stores it in the database. We need to enhance EventIterator's handling of unknown event kinds to ensure smooth processing.

Current Implementation

The current implementation extracts and checks the event kind as follows:

let key: [u8; 16] = if let Ok(key) = k.as_ref().try_into() {
    key
} else {
    return Some(Err(InvalidEvent::Key(k)));
};
let key = i128::from_be_bytes(key);
let time = Utc.timestamp_nanos((key >> 64).try_into().expect("valid i64"));
let kind_num = (key & 0xffff_ffff_0000_0000) >> 32;
let Some(kind) = EventKind::from_i128(kind_num) else {
    return Some(Err(InvalidEvent::Key(k)));
};

Suggested Enhancement

Instead of returning Some(Err(InvalidEvent::Key(k))) for unknown event kinds, we propose a change where EventIterator logs a warning message and continues to retrieve the next events until a valid event type is encountered. This approach avoids interrupting the iteration process due to unknown event kinds.

The updated implementation is as follows:

let mut key: [u8; 16];
let mut kind_num: i128;
let mut kind: Option<EventKind>;

loop {
    let (k, v) = match self.inner.next().transpose().ok().flatten() {
        Some(kv) => kv,
        None => return None,  // No more elements to iterate over
    };

    key = match k.as_ref().try_into() {
        Ok(key) => key,
        Err(_) => return Some(Err(InvalidEvent::Key(k))),
    };

    key = i128::from_be_bytes(key);
    let time = Utc.timestamp_nanos((key >> 64).try_into().expect("valid i64"));
    kind_num = (key & 0xffff_ffff_0000_0000) >> 32;
    kind = EventKind::from_i128(kind_num);

    if kind.is_none() {
        warn!("Encountered an unknown event kind: {}", kind_num);
    } else {
        break;  // Found a valid event type, exit the loop
    }
}

In this approach, EventIterator::next() continues to fetch the next event until it finds a valid event type. Upon encountering an unknown event kind, it logs a warning and continues the loop until it finds a known event type or there are no more events to iterate over.

Benefits

This improvement enhances EventIterator's resilience against unknown event kinds, ensuring the iteration process remains uninterrupted even with the introduction of new, currently unknown event kinds. The change increases overall robustness and eases the integration of new event kinds in the future.