Geo lookup returns FastlyStatus::None when not found, which is different from Compute
joeshaw opened this issue · 2 comments
When calling the geo::lookup hostcall, if there is no match for the provided IP, Viceroy returns FastlyStatus::None. Fastly Compute, however, returns no error and an empty buffer. Viceroy should match this behavior.
Compute should match Viceroy in this case. Where the empty buffer is converted to a correct type for the different SDKs?
I agree with you in principle, but doing this would be a breaking API change for the released versions of the Go SDK (see #341) because it would bail out upon hitting an error, whereas it returns no error and empty data when a lookup isn't found in Compute.
In Go, it's handled here: https://github.com/fastly/compute-sdk-go/blob/main/geo/geodata.go#L68-L71
In Rust, if there is an error returned (as Viceroy does) it just returns Ok(None): https://docs.rs/fastly/latest/src/fastly/geo.rs.html#82. If the buffer is empty it attempts to JSON decodes it, fails, and returns None: https://docs.rs/fastly/latest/src/fastly/geo.rs.html#50-51
I don't have a lot of experience with the JS SDK, but my reading is that the SDK function to get geo IP data is here: https://github.com/fastly/js-compute-runtime/blob/4f399434c0959e902df03262dfceefdc16592afe/runtime/js-compute-runtime/builtins/fastly.cpp#L52-L67 and that calls core::get_geo_info which will handle an error if it's encountered here: https://github.com/fastly/js-compute-runtime/blob/4f399434c0959e902df03262dfceefdc16592afe/runtime/js-compute-runtime/core/geo_ip.cpp#L34-L38. I'm not sure if this throws an exception or just returns an empty object.