fix: serialize controller datetimes as UTC JSON#14
fix: serialize controller datetimes as UTC JSON#14Rerowros wants to merge 1 commit intoPasarGuard:mainfrom
Conversation
WalkthroughThis pull request adds UTC datetime formatting to controller logs and JSON serialization for API requests. A new ChangesUTC Datetime Formatting and Serialization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🧹 Nitpick comments (1)
PasarGuardNodeBridge/controller.py (1)
38-38: ⚡ Quick winStrengthen the string replacement to avoid edge cases.
The
.replace("+00:00", "Z")could theoretically match "+00:00" appearing elsewhere in the ISO string, though this is unlikely. Using a more precise replacement that only targets the timezone suffix would be safer.♻️ Proposed fix: use suffix-specific replacement
- return value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") + iso_string = value.astimezone(timezone.utc).isoformat() + if iso_string.endswith("+00:00"): + return iso_string[:-6] + "Z" + return iso_string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PasarGuardNodeBridge/controller.py` at line 38, The current return uses value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") which may accidentally replace any "+00:00" substring; change it to compute the ISO string first (use value.astimezone(timezone.utc).isoformat()), then only replace the trailing timezone suffix — e.g., check if the ISO string endswith("+00:00") and if so replace that suffix with "Z" — updating the expression around value.astimezone(...).isoformat() and removing the global .replace call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PasarGuardNodeBridge/controller.py`:
- Line 37: The raise of NodeAPIError for naive datetime uses a duplicate code
(-6) already used for "Invalid proxy format"; locate the raise in controller.py
(the NodeAPIError call with detail "Naive datetime values are not supported in
JSON payloads") and change its code to a unique, unused error code (e.g., -7),
and update any centralized error registry, mappings, or tests that reference
this specific error so the new code is recognized; ensure the NodeAPIError
instantiation and any downstream handlers/logging reflect the new code.
---
Nitpick comments:
In `@PasarGuardNodeBridge/controller.py`:
- Line 38: The current return uses
value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") which may
accidentally replace any "+00:00" substring; change it to compute the ISO string
first (use value.astimezone(timezone.utc).isoformat()), then only replace the
trailing timezone suffix — e.g., check if the ISO string endswith("+00:00") and
if so replace that suffix with "Z" — updating the expression around
value.astimezone(...).isoformat() and removing the global .replace call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15b0e201-bc1d-4f6b-b0d2-7b876b01440b
📒 Files selected for processing (1)
PasarGuardNodeBridge/controller.py
| def _json_safe(value: Any) -> Any: | ||
| if isinstance(value, datetime): | ||
| if value.tzinfo is None or value.utcoffset() is None: | ||
| raise NodeAPIError(code=-6, detail="Naive datetime values are not supported in JSON payloads") |
There was a problem hiding this comment.
Error code conflict: code=-6 is already used.
The error code -6 is already defined at line 119 for "Invalid proxy format". Using the same code for "Naive datetime values are not supported" creates ambiguity when handling errors.
🐛 Proposed fix: use a unique error code
- raise NodeAPIError(code=-6, detail="Naive datetime values are not supported in JSON payloads")
+ raise NodeAPIError(code=-7, detail="Naive datetime values are not supported in JSON payloads")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PasarGuardNodeBridge/controller.py` at line 37, The raise of NodeAPIError for
naive datetime uses a duplicate code (-6) already used for "Invalid proxy
format"; locate the raise in controller.py (the NodeAPIError call with detail
"Naive datetime values are not supported in JSON payloads") and change its code
to a unique, unused error code (e.g., -7), and update any centralized error
registry, mappings, or tests that reference this specific error so the new code
is recognized; ensure the NodeAPIError instantiation and any downstream
handlers/logging reflect the new code.
|
we have no datetime variable in node response |
|
Thanks, you're right. I rechecked the node responses |
2 participants
Summary
This pull request makes datetime handling in controller JSON payloads stricter and more predictable.
Timezone-aware
datetimevalues are now serialized as UTC with aZsuffix. Naive datetimes are rejected before they can be sent in JSON payloads.JSON Payload Serialization
PasarGuardNodeBridge/controller.py.PasarGuardNodeBridge/controller.py.PasarGuardNodeBridge/controller.py.Why This Is Better
Naive datetimes are unsafe in API payloads because their meaning depends on local process timezone assumptions. That can cause subtle mismatches between panel, node bridge, and node behavior.
By requiring timezone-aware datetime values and serializing them as UTC, the bridge sends payloads with unambiguous timestamps. This is easier to debug and safer for integrations that compare events across services.