Align e2e-cli types with sdk-e2e-tests#533
Merged
MichaelGHSeg merged 2 commits intomasterfrom Feb 26, 2026
Merged
Conversation
Add missing field extraction: timestamp, category, context, integrations. Wire timestamp, messageId, context, and integrations through to the Java SDK MessageBuilder for full fidelity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e2e-cli/src/main/kotlin/cli/Main.kt
Outdated
Comment on lines
189
to
199
| private fun parseTimestamp(iso: String): Date { | ||
| val format = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", Locale.US) | ||
| format.timeZone = TimeZone.getTimeZone("UTC") | ||
| return try { | ||
| format.parse(iso)!! | ||
| } catch (_: Exception) { | ||
| // Fallback: try without millis | ||
| val fallback = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.US) | ||
| fallback.timeZone = TimeZone.getTimeZone("UTC") | ||
| fallback.parse(iso)!! | ||
| } |
There was a problem hiding this comment.
I am always suspicious of the jvm and date functions so I gave this function to claude:
This Kotlin timestamp parsing function has a few issues:
Problems:
- Uses deprecated SimpleDateFormat - java.time.Instant and java.time.format.DateTimeFormatter are preferred in modern code
- Force unwraps with !! - Can throw NPE if parsing fails in both attempts
- Silent exception swallowing - The catch block ignores the original exception, making debugging harder
- Creates formatters on every call - SimpleDateFormat creation is expensive
Potential runtime issue:
If the fallback also fails to parse, you'll get a NullPointerException from the !! operator with no context about the original parsing error.
I think we address at least the issue where we don't show the parse error.
Contributor
Author
There was a problem hiding this comment.
New code is much simpler, better?
didiergarcia
previously approved these changes
Feb 26, 2026
Instant.parse() handles all ISO 8601 formats natively, avoids deprecated SimpleDateFormat, and throws DateTimeParseException with clear context on invalid input instead of returning null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
didiergarcia
approved these changes
Feb 26, 2026
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
2 participants
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
timestamp,category,context,integrationstimestamp,messageId,context, andintegrationsthrough to the Java SDKMessageBuildersdk-e2e-tests/src/cli/types.tsTest plan
mvn package -pl e2e-cli -am -DskipTestsbuilds cleanly./e2e-cli/run-e2e.sh🤖 Generated with Claude Code