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

Counterintuitive Accept-Header for the json-query monitor #4491

Open
1 task done
bcarlock-mycarrier opened this issue Feb 14, 2024 · 13 comments
Open
1 task done

Counterintuitive Accept-Header for the json-query monitor #4491

bcarlock-mycarrier opened this issue Feb 14, 2024 · 13 comments
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added type:enhance-existing feature wants to enhance existing monitor

Comments

@bcarlock-mycarrier
Copy link

📑 I have found these related issues/pull requests

I wasn't able to find any similar issues.

🛡️ Security Policy

Description

Any time I attempt to use a query I receive Unexpected token < in JSON at position 0

👟 Reproduction steps

Create a monitor that uses header authentication with the JSON Query monitor.
Given the following JSON response:
{"importantnumber":98,"number":0,"failures":8276,"warnings":0,"lastprocessed":"2024-02-14T00:48:15.837Z"}
and the JSON query ($.importantnumber < 1000), the response should be "true"
image
image

👀 Expected behavior

Monitor should show the status as up.

😓 Actual Behavior

Monitor throws an error and cannot get status
image

🐻 Uptime-Kuma Version

1.23.11

💻 Operating System and Arch

Ubuntu 22.04 (Kubernetes)

🌐 Browser

Brave v1.62.162, Vivaldi 6.5.3206.61

🖥️ Deployment Environment

  • Runtime: Docker/Kubernetes
  • Database: sqlite/embedded
  • Filesystem used to store the database on: Not sure
  • number of monitors: 3

📝 Relevant log output

2024-02-13T19:45:50-06:00 [MONITOR] WARN: Monitor #7 'PushTracking': Failing: Unexpected token < in JSON at position 0 | Interval: 60 seconds | Type: json-query | Down Count: 0 | Resend Interval: 0
2024-02-13T19:46:23-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:46:32-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:46:50-06:00 [MONITOR] WARN: Monitor #7 'PushTracking': Failing: Unexpected token < in JSON at position 0 | Interval: 60 seconds | Type: json-query | Down Count: 0 | Resend Interval: 0
2024-02-13T19:47:23-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:47:32-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:47:50-06:00 [MONITOR] WARN: Monitor #7 'PushTracking': Failing: Unexpected token < in JSON at position 0 | Interval: 60 seconds | Type: json-query | Down Count: 0 | Resend Interval: 0
2024-02-13T19:48:23-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:48:51-06:00 [MONITOR] WARN: Monitor #7 'PushTracking': Failing: Unexpected token < in JSON at position 0 | Interval: 60 seconds | Type: json-query | Down Count: 0 | Resend Interval: 0
2024-02-13T19:49:23-06:00 [RATE-LIMIT] INFO: remaining requests: 60
2024-02-13T19:49:51-06:00 [MONITOR] WARN: Monitor #7 'PushTracking': Failing: Unexpected token < in JSON at position 0 | Interval: 60 seconds | Type: json-query | Down Count: 0 | Resend Interval: 0
@bcarlock-mycarrier bcarlock-mycarrier added the bug Something isn't working label Feb 14, 2024
@CommanderStorm
Copy link
Collaborator

Before we dive into a deeper reproduction, let's cover the easy cases first:

Have you tried looking at the URL you are trying to monitor in a different window?
Might there be a typo in there?
If, for example, a <html... or a <xml... page would be returned instead of JSON, the error message you are seeing would fit.

@CommanderStorm CommanderStorm added question Further information is requested area:monitor Everything related to monitors labels Feb 14, 2024
@bcarlock-mycarrier
Copy link
Author

Before we dive into a deeper reproduction, let's cover the easy cases first:

Have you tried looking at the URL you are trying to monitor in a different window? Might there be a typo in there? If, for example, a <html... or a <xml... page would be returned instead of JSON, the error message you are seeing would fit.

Yes, it's been double checked and the json response I posted as an example is a real response generated from CURL (with some obfuscation, which honestly was kind of pointless)
Here's an example from this morning:
image

As you can see, we get a valid JSON response. I also checked the authorization header by changing the endpoint to just a basic HTTPS monitor, which succeeds.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Feb 14, 2024

I tried reproducing this issue and cannot.

=> please verify that all parameters are exactly equal (auth method, headers, URL, required response-type, request body-encoding, …). I am assuming you are getting an http/xml message.

Steps I did to reproduce this:

server.go

package main

import (
    "fmt"
    "net/http"
)

func hello(w http.ResponseWriter, req *http.Request) {
    fmt.Fprintf(w, "{\"importantnumber\":98,\"number\":0,\"failures\":8276,\"warnings\":0,\"lastprocessed\":\"2024-02-14T00:48:15.837Z\"}")
}

func main() {
    http.HandleFunc("/", hello)
    http.ListenAndServe(":8090", nil)
}

docker run -it -d --network=host louislam/uptime-kuma:1.23.11 # the --network is just to not have to expose the network to said container
go run server.go

image

@CommanderStorm CommanderStorm added cannot-reproduce help and removed bug Something isn't working labels Feb 14, 2024
@bcarlock-mycarrier
Copy link
Author

bcarlock-mycarrier commented Feb 15, 2024

You can see from my screenshot earlier that the response is JSON. Is there anything you'd like for me to do to demonstrate?
I showed the actual request, the actual endpoint and actual response via curl...

@bcarlock-mycarrier
Copy link
Author

Our Uptime-Kuma instance is running behind Istio. Are there any known issues running in that configuration? This is the only monitor type we've had any issues with.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Feb 15, 2024

Is there anything you'd like for me to do to demonstrate?

Not demonstrate, but verify, that these parameters are the same the between the monitor as with curl.
You are blurring a part of your curl command associated with a header.

image

@CommanderStorm
Copy link
Collaborator

Uptime-Kuma instance is running behind Istio

Unlikely. Reverse proxies only affect incoming traffic, not outgoing, or is Istio different?
We have not had an issue mentioning said service mesh and I don't have any experience with this area.

@CommanderStorm
Copy link
Collaborator

Please also try executing the above curl command in the container itsself (see
https://github.com/louislam/uptime-kuma/wiki/Troubleshooting)

Maybe this has different results from your local machine

@bcarlock-mycarrier
Copy link
Author

Uptime-Kuma instance is running behind Istio

Unlikely. Reverse proxies only affect incoming traffic, not outgoing, or is Istio different? We have not had an issue mentioning said service mesh and I don't have any experience with this area.

Kind of, the gateway acts as a reverse proxy, but the istio sidecar is technically a transparent full proxy. I've never had issues with Istio mangling requests in or out, but I figured it was worth asking. My results with curl (below) pretty definitively prove that the issue isn't Istio though.

Please also try executing the above curl command in the container itsself (see https://github.com/louislam/uptime-kuma/wiki/Troubleshooting)

Maybe this has different results from your local machine

Curl from the uptime-kuma container returns results consistent with my local machine. See below.
image

I've been trying to figure out a way to get uptime-kuma to actually show the response it receives upon an error. When I switch to HTTP(s) - Keyword I get this:
image

Which led me to add "Accept: application/json" to the headers. After adding that header it's working. If the error was handled better (like in Keyword) I wouldn't have ever opened this issue.

@CommanderStorm
Copy link
Collaborator

Interesting.
We might want to add application/json here:

"Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9",

As far as I understand our options, application/json should be accepted in any case since it should match */*.

Maybe we should also change the Accepts header if json-query is chosen, as f.ex. accepting images does not really make sense in this context, only json-ish and text-ish formats.

Headsup to @chakflying given the planned work in #3919

A more long-term symptom-fix would be to allow expanding error messages with additional details (i.e. click on the error message => see the whole request).

@CommanderStorm CommanderStorm changed the title JSON Query Unexpected token < in JSON at position 0 Counterintuitive Accept-Header for the json-query monitor Feb 15, 2024
@bcarlock-mycarrier
Copy link
Author

bcarlock-mycarrier commented Feb 15, 2024

That makes a ton of sense, but shouldn't the json-query job only accept application/json? Considering it's explicitly specified in the monitor name?

@CommanderStorm
Copy link
Collaborator

Counter question: Why should it only accept json?

See #4423 for a related issue where processing text is wanted...

@bcarlock-mycarrier
Copy link
Author

Just because we can parse plain text with jsonata doesn't mean we should. I'd vote for a monitor that parses via regex for something plain text. It allows for implicit accepts headers in each job and reduces the amount of documentation necessary for edge cases like parsing plain text in a json query monitor. TL;DR: When I use a monitor that says JSON Query, I expect it to work exclusively for JSON. If I have something like a regex parser I will expect it to be able to parse effectively any HTTP text response, including JSON.

@CommanderStorm CommanderStorm added feature-request Request for new features to be added type:enhance-existing feature wants to enhance existing monitor and removed question Further information is requested help labels Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added type:enhance-existing feature wants to enhance existing monitor
Projects
None yet
Development

No branches or pull requests

2 participants