WeatherAPI Kotlinx Serialization Fails on Missing Fields in Weather Response

[coderGtm]

Description:

When using the weather command in the app after #428, deserialization of the API response fails with io.ktor.serialization.JsonConvertException if certain fields are missing from the JSON payload. This is due to the Kotlin data models (Current, Condition, TodayForecast, etc.) having required (non-null, no-default) properties that the API sometimes omits, especially in edge cases or when querying specific endpoints.

Observed Errors:

• Missing fields in Condition (e.g., icon, code) cause exceptions at $.current.condition.
• Missing fields in Current (e.g., last_updated_epoch, is_day, wind_degree, etc.) cause exceptions at $.current.
• Missing astro object in TodayForecast causes exceptions at $.forecast.forecastday[0].

Example Exception:

io.ktor.serialization.JsonConvertException: Illegal input: Fields [icon, code] are required for type with serial name 'com.coderGtm.yantra.commands.weather.Condition', but they were missing at path: $.current.condition

Root Cause:

• Kotlinx Serialization treats missing required fields as errors.
• WeatherAPI responses sometimes omit fields, returning only a subset of the expected data.

Impact:

• Weather command fails and does not return results.
• Application logs serialization exceptions.

Steps to Reproduce:

1. Call the weather command with a location or parameters that trigger a minimal or incomplete API response. ( I used weather vadodara )
2. Observe the exception in logs indicating missing required fields.

Expected Behavior:

• The weather command should succeed even if some fields are missing in the API response.
• The command should handle missing data gracefully.

Actual Behavior:

• The command fails with a serialization exception if any required field is missing.

Solution? What do you think can be done here @sebastinto ? I think making the fields nullable with a default value is one solution which I tried.

[sebastinto]

Good catch!

I think making the fields nullable with a default value is one solution which I tried.

That's what I'd do!

With vadodara as the location, it seems like some properties of the Current data class:

@Serializable
- @SerialName("short_rad") val shortRad: Double,
- @SerialName("diff_rad") val diffRad: Double,
- val dni: Double,
- val gti: Double,
+ @SerialName("short_rad") val shortRad: Double? = null,
+ @SerialName("diff_rad") val diffRad: Double? = null,
+ val dni: Double? = null,
+ val gti: Double? = null

as well as DayForecast data class need to be nullable

- @SerialName("air_quality") val airQuality: AirQuality
+ @SerialName("air_quality") val airQuality: AirQuality? = null

[sebastinto]

I can submit a quick bugfix PR if you'd like @coderGtm ?

[coderGtm]

Thanks for the PR @sebastinto I have merged it. Do you think we should make all of the params nullable so it doesn't fail in any case?

[sebastinto]

Thanks for the PR @sebastinto I have merged it. Do you think we should make all of the params nullable so it doesn't fail in any case?

@coderGtm The "proper" way to go about it would be to respect the API contract and only set fields that are not guaranteed to be present as nullable. I actually reviewed the documentation and I think we should be fine, but can't guarantee I didn't miss anything since I went over it quickly.

Ultimately, making all fields nullable would definitely ensure the error doesn't happen again!

[coderGtm]

Alright then let's keep it as it is. I saw the docs and it looks like the missing data is actually omitted as it was for paid plan users only, so the api was not misbehaving actually. We should be fine with your current implementation. 💪🏻