Skip to content

New feature: enable rustls-no-provider - #201

Draft
yawn wants to merge 3 commits into
PostHog:mainfrom
yawn:feat/tls-no-provider
Draft

New feature: enable rustls-no-provider#201
yawn wants to merge 3 commits into
PostHog:mainfrom
yawn:feat/tls-no-provider

Conversation

@yawn

@yawn yawn commented Aug 10, 2026

Copy link
Copy Markdown

💡 Motivation and Context

This avoids automatically pulling in aws-lc-rs which is quite heavyweight, requires a C toolchain etc.

Note that this change is breaking for everybody who uses this crate with default-features = false. Everybody else is unaffected.

💚 How did you test it?

Added an integration test and optionally (extra commit, feel free to drop it) a build/ci test.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

@yawn
yawn requested a review from a team as a code owner August 10, 2026 10:32
@marandaneto

Copy link
Copy Markdown
Member

thanks @yawn will take a look at it

@marandaneto

Copy link
Copy Markdown
Member

we're planning the v1 major release with a few breaking changes, and since this has a breaking change if default-features = false we'd need to fit this in there, so people follow a proper migration before breaking things at runtime

Comment thread .github/workflows/ci.yaml
cache-key: tls-no-provider
command: >-
cargo build --verbose --no-default-features --features async-client,error-tracking,capture-v1,tls-no-provider
&& ! cargo tree --package posthog-rs --edges normal --invert aws-lc-sys --no-default-features --features async-client,error-tracking,capture-v1,tls-no-provider

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: Make the dependency-tree assertion fail closed\n\nNegating cargo tree makes any failure—including registry, dependency-resolution, or argument errors—look like proof that aws-lc-sys is absent, so this regression guard can pass silently. Please run cargo tree as a separately checked command, then inspect its successful output for aws-lc-sys.

Comment thread .github/workflows/ci.yaml
- name: E2E test
cache-key: e2e
command: cargo test --verbose --features e2e-test --no-default-features
command: cargo test --verbose --features e2e-test,tls --no-default-features

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Update the documented E2E command\n\nCI now enables tls for E2E tests, but CONTRIBUTING.md still documents --features e2e-test --no-default-features. That leaves manual E2E runs without an HTTPS backend, so captures can fail while the test appears successful. Please update the documented command to include tls.

@marandaneto

Copy link
Copy Markdown
Member

also:

Commits must have verified signatures

you have to sign the commits

@marandaneto
marandaneto marked this pull request as draft August 10, 2026 10:50
@marandaneto

Copy link
Copy Markdown
Member

moving to draft until we figure this out

@yawn

yawn commented Aug 10, 2026

Copy link
Copy Markdown
Author

also:

Commits must have verified signatures

you have to sign the commits

Oh, was that in the PR template? Sorry, must have overlooked it ...

@yawn

yawn commented Aug 10, 2026

Copy link
Copy Markdown
Author

moving to draft until we figure this out

I'd argue that this could still be a minor semver bump. It's not a breaking API change, it's just a non-breaking change (a simple remediation exists without functional implications) for a specific subset of users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants