Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

Comments

refactor!: make S2DateTime conversion from time::OffsetDateTime fallible#312

Merged
quettabit merged 1 commit intomainfrom
qb/s2dt-fallible
Feb 15, 2026
Merged

refactor!: make S2DateTime conversion from time::OffsetDateTime fallible#312
quettabit merged 1 commit intomainfrom
qb/s2dt-fallible

Conversation

@quettabit
Copy link
Member

No description provided.

@quettabit quettabit changed the title [WIP] refactor!: make conversion from time::OffsetDateTime to S2DateTime as fallible Feb 15, 2026
@quettabit quettabit changed the title refactor!: make conversion from time::OffsetDateTime to S2DateTime as fallible refactor!: make S2DateTime conversion from time::OffsetDateTime fallible Feb 15, 2026
@quettabit quettabit marked this pull request as ready for review February 15, 2026 07:55
@quettabit quettabit requested a review from a team as a code owner February 15, 2026 07:55
@greptile-apps
Copy link

greptile-apps bot commented Feb 15, 2026

Greptile Summary

Changed S2DateTime conversion from time::OffsetDateTime to be fallible (using TryFrom instead of From), adding validation that the datetime can be formatted as RFC 3339. This prevents potential runtime panics from out-of-range years (outside -9999 to 9999).

  • Implemented TryFrom<OffsetDateTime> for S2DateTime with RFC 3339 format validation
  • Updated all conversion sites (AccessTokenInfo, StreamInfo in types.rs, and operations in ops.rs) to use fallible conversion with ? operator
  • Updated test code to use try_into() instead of into()
  • Made error messages consistent: "not a valid RFC 3339 datetime"

This is a breaking change that improves type safety by catching invalid datetimes at conversion time rather than during display/serialization.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The refactoring is systematic and complete, properly converting all usage sites from infallible to fallible conversion. The validation logic is sound (RFC 3339 format validation), error handling is consistent, and the breaking change is appropriately marked with ! in the PR title
  • No files require special attention

Important Files Changed

Filename Overview
src/types.rs Changed S2DateTime conversion from From to TryFrom with RFC3339 validation, updated dependent conversions in AccessTokenInfo and StreamInfo
src/ops.rs Updated list_streams and create_stream to use try_into() for fallible StreamInfo conversion
tests/account_ops.rs Updated test to use try_into() for S2DateTime conversion from OffsetDateTime

Flowchart

flowchart TD
    A[time::OffsetDateTime] -->|TryFrom::try_from| B{Validate RFC 3339}
    B -->|Can format| C[S2DateTime created]
    B -->|Cannot format| D[ValidationError]
    
    E[API Response] -->|parse| F[api::stream::StreamInfo]
    F -->|TryFrom| G{Convert created_at}
    G -->|Success| H{Convert deleted_at}
    G -->|Error| I[ValidationError]
    H -->|Success| J[StreamInfo]
    H -->|Error| I
    
    K[list_streams] -->|collect| L{Convert Vec of StreamInfo}
    L -->|All succeed| M[Page of StreamInfo]
    L -->|Any fails| N[S2Error]
    
    C -.->|Used in| G
    C -.->|Used in| H
Loading

Last reviewed commit: ac854d8

@quettabit quettabit merged commit e2a4cda into main Feb 15, 2026
6 checks passed
@quettabit quettabit deleted the qb/s2dt-fallible branch February 15, 2026 07:59
@github-actions github-actions bot mentioned this pull request Feb 15, 2026
shikhar pushed a commit that referenced this pull request Feb 15, 2026
## 🤖 New release

* `s2-sdk`: 0.23.8 -> 0.24.0 (⚠ API breaking changes)

### ⚠ `s2-sdk` breaking changes

```text
--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/inherent_method_missing.ron

Failed in:
  CreateBasinInput::with_idempotency_token, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:913
  CreateStreamInput::with_idempotency_token, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:2583

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field idempotency_token of struct CreateBasinInput, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:882
  field idempotency_token of struct CreateStreamInput, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:2561

--- failure struct_pub_field_now_doc_hidden: pub struct field is now #[doc(hidden)] ---

Description:
A pub field of a pub struct is now marked #[doc(hidden)] and is no longer part of the public API.
        ref: https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/struct_pub_field_now_doc_hidden.ron

Failed in:
  field CreateBasinInput.idempotency_token in file /tmp/.tmp5yfqnq/s2-sdk-rust/src/types.rs:868
  field CreateStreamInput.idempotency_token in file /tmp/.tmp5yfqnq/s2-sdk-rust/src/types.rs:2539
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.24.0] - 2026-02-15

### Features

- Add accessors for `AppendRecord`
([#305](#305))
- [**breaking**] Add lower bounds for `max_batch_bytes` and
`max_batch_records`
([#309](#309))
- [**breaking**] Reduce default `max_unacked_bytes` to `5MiB`
([#311](#311))

### Refactor

- Replace `reqwest` with `hyper-util` and add client pooling
([#298](#298))
- [**breaking**] Make `idempotency_token` private
([#306](#306))
- [**breaking**] Remove unnecessary `Result` from
`with_max_unacked_batches`
([#307](#307))
- Remove unnecessary compression for GET and DELETE requests
([#308](#308))
- Rename fields, methods, and vars related to `RetryBackoff`
([#310](#310))
- [**breaking**] Make `S2DateTime` conversion from
`time::OffsetDateTime` fallible
([#312](#312))

### Testing

- Metrics
([#297](#297))
- Basin & stream api
([#300](#300))

### Miscellaneous Tasks

- Bump dependencies
([#296](#296))
- Dep updates
([#314](#314))

<!-- generated by git-cliff -->
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant