Meshbir parsing#378
Merged
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances the robustness of MeshBir message parsing by adding error handling and filtering for malformed JSON in flushMessageQueue, and corrects a typo in the meshbir route tests to ensure invalid message type is validated properly. Class diagram for updated MeshBir message parsing in flushMessageQueueclassDiagram
class flushMessageQueue {
+Promise<MeshBirMessage[]> flushMessageQueue(redis: Redis)
}
class MeshBirMessage
flushMessageQueue --> MeshBirMessage: returns array of
flushMessageQueue : +try/catch JSON.parse
flushMessageQueue : +console.error on parse failure
flushMessageQueue : +filters out undefined messages
Flow diagram for enhanced error handling in flushMessageQueueflowchart TD
A["Retrieve messages from Redis"] --> B["Iterate over messages"]
B --> C["Try to parse JSON"]
C -->|Success| D["Add to result array"]
C -->|Failure| E["Log error and skip message"]
D & E --> F["Filter out undefined messages"]
F --> G["Sort messages by time"]
G --> H["Return sorted array"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis pull request adds robust JSON parsing error handling to the meshbir queue processing by wrapping Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `apps/fetcher/src/app/trackers/meshbir.ts:159-166` </location>
<code_context>
// Return older messages first
return (messages as string[])
- .map((json) => JSON.parse(json) as MeshBirMessage)
+ .map((json) => {
+ try {
+ return JSON.parse(json) as MeshBirMessage;
+ } catch {
+ console.error(`Error parsing meshbir ${json}`);
+ return undefined;
+ }
+ })
</code_context>
<issue_to_address>
**suggestion:** Consider logging the error object for more context when JSON.parse fails.
Logging the error object alongside the JSON string will make it easier to identify the cause of parsing failures.
```suggestion
.map((json) => {
try {
return JSON.parse(json) as MeshBirMessage;
} catch (err) {
console.error(`Error parsing meshbir ${json}`, err);
return undefined;
}
})
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+159
to
+166
| .map((json) => { | ||
| try { | ||
| return JSON.parse(json) as MeshBirMessage; | ||
| } catch { | ||
| console.error(`Error parsing meshbir ${json}`); | ||
| return undefined; | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
suggestion: Consider logging the error object for more context when JSON.parse fails.
Logging the error object alongside the JSON string will make it easier to identify the cause of parsing failures.
Suggested change
| .map((json) => { | |
| try { | |
| return JSON.parse(json) as MeshBirMessage; | |
| } catch { | |
| console.error(`Error parsing meshbir ${json}`); | |
| return undefined; | |
| } | |
| }) | |
| .map((json) => { | |
| try { | |
| return JSON.parse(json) as MeshBirMessage; | |
| } catch (err) { | |
| console.error(`Error parsing meshbir ${json}`, err); | |
| return undefined; | |
| } | |
| }) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/fetcher/src/app/trackers/meshbir.ts(1 hunks)apps/fxc-server/src/app/routes/meshbir.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/fxc-server/src/app/routes/meshbir.test.ts (1)
apps/fxc-server/src/app/routes/meshbir.ts (1)
parseMessage(46-55)
apps/fetcher/src/app/trackers/meshbir.ts (1)
libs/common-node/src/lib/meshtbir.ts (1)
MeshBirMessage(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Sourcery review
- GitHub Check: Analyze (javascript)
- GitHub Check: build (22.x)
🔇 Additional comments (1)
apps/fxc-server/src/app/routes/meshbir.test.ts (1)
69-69: LGTM! Typo corrected.The typo fix from 'unkown' to 'unknown' improves test readability without changing the test behavior.
Comment on lines
+159
to
+167
| .map((json) => { | ||
| try { | ||
| return JSON.parse(json) as MeshBirMessage; | ||
| } catch { | ||
| console.error(`Error parsing meshbir ${json}`); | ||
| return undefined; | ||
| } | ||
| }) | ||
| .filter((msg) => msg !== undefined) |
There was a problem hiding this comment.
Excellent error handling, but avoid logging sensitive data.
The try-catch wrapper around JSON.parse effectively prevents crashes from malformed queue entries, and filtering out undefined values maintains clean data flow.
However, logging the entire JSON string exposes sensitive user data including location coordinates and user identifiers.
Apply this diff to log parse errors without exposing PII:
.map((json) => {
try {
return JSON.parse(json) as MeshBirMessage;
} catch {
- console.error(`Error parsing meshbir ${json}`);
+ console.error(`Error parsing meshbir message - invalid JSON format`);
return undefined;
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .map((json) => { | |
| try { | |
| return JSON.parse(json) as MeshBirMessage; | |
| } catch { | |
| console.error(`Error parsing meshbir ${json}`); | |
| return undefined; | |
| } | |
| }) | |
| .filter((msg) => msg !== undefined) | |
| .map((json) => { | |
| try { | |
| return JSON.parse(json) as MeshBirMessage; | |
| } catch { | |
| console.error(`Error parsing meshbir message - invalid JSON format`); | |
| return undefined; | |
| } | |
| }) | |
| .filter((msg) => msg !== undefined) |
🤖 Prompt for AI Agents
In apps/fetcher/src/app/trackers/meshbir.ts around lines 159 to 167, the catch
block logs the entire JSON string which can expose PII; change the error logging
to avoid printing the raw payload by logging a non-sensitive summary (e.g., an
error message plus a short fixed-length prefix, a truncated length, or a hash of
the payload) and include the parse error message or stack for debugging; ensure
the log never contains full coordinates or user identifiers and keep the record
of the failure (timestamp, queue id if available, truncated or hashed payload,
and error) so you can debug without exposing sensitive data.
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
1 participant
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 by Sourcery
Improve meshbir message parsing and correct test typo
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests