Repository navigation
fix(cf): make timezone_name optional - #1099
Open
PedroRosalba wants to merge 1 commit into
Open
PedroRosalba wants to merge 1 commit into
PedroRosalba wants to merge 1 commit into
Conversation
`request.cf.timezone` is optional in workers-types (geolocation data can be missing for a client IP, and tooling-supplied `cf` objects often omit it), but worker-sys declared the getter as `Result<String>` and `Cf::timezone_name()` unwrapped it, so a request without a timezone panicked in both `--dev` and `--release` builds. Declare the getter as `Result<Option<String>>` and return `Option<String>` from `Cf::timezone_name()`, following cloudflare#776 / cloudflare#777 which made `as_organization` and `asn` optional the same way. The feature-gated `Cf::timezone()` already propagated the glue error with `?`; it now returns the typed `Error::TimezoneError` when the field is absent. Adds a `/cf/timezone-name` route and a test that dispatches a request with and without `cf.timezone`.
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
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.
Problem
request.cf.timezoneis optional in workers-types (geolocation data can be missing for a client IP, and tooling-suppliedcfobjects often omit it), but worker-sys declares the getter asResult<String>andCf::timezone_name()unwraps it. A request without a timezone panics in both--devand--releasebuilds.Fix
Declare the getter as
Result<Option<String>>and returnOption<String>fromCf::timezone_name(), following #776 / #777 which madeas_organizationandasnoptional the same way. The feature-gatedCf::timezone()already propagated the glue error with?; it now returns the typedError::TimezoneErrorwhen the field is absent.This changes the return type of
timezone_name()fromStringtoOption<String>, the same patch-level treatment #777 received.Tests
Adds a
/cf/timezone-nameroute and a spec that dispatches a request with and withoutcf.timezone. Panics onmain, passes with this change.Tested on macOS (Apple Silicon) only.