Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Program panics when serde_json is built with arbitrary_precision feature #14

Open
RichardWGNR opened this issue Sep 19, 2024 · 2 comments

Comments

@RichardWGNR
Copy link

RichardWGNR commented Sep 19, 2024

Description

Program panics when serde_json is built with arbitrary_precision feature

thread 'actix-server worker 0' panicked at src\http_services\api\dashboard\summary.rs:11:14:
called `Result::unwrap()` on an `Err` value: Client(ClientError { message: "failed to parse JSON response from server", source: Some(reqwest::Error { kind: Decode, source: Error("invalid type: map, expected f64", line: 0, column: 0) }) })
stack backtrace:

To reproduce

  1. cargo.toml
[dependencies]
serde_json = { version = "1.0.117", features = ["arbitrary_precision"] }
prometheus-http-query = { version = "0.8.3" }
  1. Then try querying any metric via Client::query().await

Cause

  1. This is due to the way serde_json handles numeric values with “arbitrary_precision” feature.
  2. The current logic forces serde_json to cast the String type to Float64:
    #[serde(deserialize_with = "de::deserialize_f64")]

My opinion

This is a bug in the current crate.

I believe there is no need to convert String to F64, as that is the prerogative of those who really need it.

On the other hand, if contributors wish not to fix this, the correct operation of the crate with arbitrary_precision feature should be implemented.

@puetzp
Copy link
Owner

puetzp commented Sep 20, 2024

Thank you for submitting this issue.

As per several issues in the serde_json crate (e.g. here and here), the problem seems to be caused by using an enum to deserialize both plain f64 and floats stored in a string.

I will have to spend some more time to dig into the Prometheus API and see in which cases it returns a string or a float. As far as I remember most response types return a float within a string because a query could in theory return a string value instead of a scalar, as documented here. But none of the the Prometheus endpoints ever return a string that does not contain a float.

Be that as it may, maybe we can just get rid of the untagged enum that is currently used in deserialize_f64. Using untagged enums is certainly not the most efficient way to deserialize responses anyway. This should also resolve this issue if it is indeed related to the way serde_json handlers enums when arbitrary_precision is enabled.

I believe there is no need to convert String to F64, as that is the prerogative of those who really need it.

I think I will stick to the current strategy of deserializing to f64 in cases where the value cannot be anything else. As far as I can see this does apply to most HTTP API endpoints, except query and query_range.

@puetzp
Copy link
Owner

puetzp commented Sep 27, 2024

Update: I changed the API a little to eliminate a "helper" enum in the deserialize_f64function. However this alone does not alleviate the issue. I suspect I would have to change more parts that help to deserialize Prometheus API responses, e.g. the internally tagged Rule enum. This enum deserializes responses such as:

{                                                                                                                                                                                                                                                       
  "groups": [                                                                                                                                                                                                                                           
    {                                                                                                                                                                                                                                                   
      "rules": [                                                                                                                                                                                                                                        
        {                                                                                                                                                                                                                                               
          "alerts": [                                                                                                                                                                                                                                   
            {                                                                                                                                                                                                                                           
              "activeAt": "2018-07-04T20:27:12.60602144+02:00",                                                                                                                                                                                         
              "annotations": {                                                                                                                                                                                                                          
                "summary": "High request latency"                                                                                                                                                                                                       
              },                                                                                                                                                                                                                                        
              "labels": {                                                                                                                                                                                                                               
                "alertname": "HighRequestLatency",                                                                                                                                                                                                      
                "severity": "page"                                                                                                                                                                                                                      
              },                                                                                                                                                                                                                                        
              "state": "firing",                                                                                                                                                                                                                        
              "value": "1e+00"                                                                                                                                                                                                                          
            }                                                                                                                                                                                                                                           
          ],                                                                                                                                                                                                                                            
          "annotations": {                                                                                                                                                                                                                              
            "summary": "High request latency"                                                                                                                                                                                                           
          },                                                                                                                                                                                                                                            
          "duration": 600,                                                                                                                                                                                                                              
          "health": "ok",                                                                                                                                                                                                                               
          "labels": {                                                                                                                                                                                                                                   
            "severity": "page"                                                                                                                                                                                                                          
          },                                                                                                                                                                                                                                            
          "name": "HighRequestLatency",                                                                                                                                                                                                                 
          "query": "job:request_latency_seconds:mean5m{job=\"myjob\"} > 0.5",                                                                                                                                                                           
          "type": "alerting",                                                                                                                                                                                                                           
          "evaluationTime": 0.000312805,                                                                                                                                                                                                                
          "lastEvaluation": "2023-10-05T19:51:25.462004334+02:00",                                                                                                                                                                                      
          "keepFiringFor": 60                                                                                                                                                                                                                           
        },                                                                                                                                                                                                                                              
        {                                                                                                                                                                                                                                               
          "health": "ok",                                                                                                                                                                                                                               
          "name": "job:http_inprogress_requests:sum",                                                                                                                                                                                                   
          "query": "sum by (job) (http_inprogress_requests)",                                                                                                                                                                                           
          "type": "recording",                                                                                                                                                                                                                          
          "evaluationTime": 0.000256946,                                                                                                                                                                                                                
          "lastEvaluation": "2023-10-05T19:51:25.052982522+02:00"                                                                                                                                                                                       
        }                                                                                                                                                                                                                                               
      ],                                                                                                                                                                                                                                                
      "file": "/rules.yaml",                                                                                                                                                                                                                            
      "interval": 60,                                                                                                                                                                                                                                   
      "limit": 0,                                                                                                                                                                                                                                       
      "name": "example",                                                                                                                                                                                                                                
      "evaluationTime": 0.000267716,                                                                                                                                                                                                                    
      "lastEvaluation": "2023-10-05T19:51:25.052974842+02:00"                                                                                                                                                                                           
    }                                                                                                                                                                                                                                                   
  ]                                                                                                                                                                                                                                                     
}

In this instance members of the rules array are deserialized based on the value of the type parameters, recording or alerting. An internally tagged enum is the most straightforward way to deserialize such a response. Based on the various issues filed in serde_json and the failing unit tests in this crate when arbitrary_precision is enabled, this internally tagged enum seems to be part of the issue.

However I'm hesitant to change the deserialize procedure here because a different implementation that does not use an enum (but e.g. an intermediary HashMap instead) would almost certainly be way less efficient.

I will think this over again soon, but for now this issue seems to be blocked on this which in turn depends on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants