jsdelivr / globalping-cli

A simple CLI tool to run networking commands remotely from hundreds of globally distributed servers
Mozilla Public License 2.0
144 stars 13 forks source link

Improve http output #58

Closed didil closed 1 year ago

didil commented 1 year ago

Fixes #55 For http GET requests:

jimaek commented 1 year ago

@MartinKolarik Not sure if this is helpful to anyone. The output is somewhat a mess now. At the least we should prefix the headers with < like curl does to differentiate between headers and body.

And what a user is supposed to do if they actually want to get the body without headers?

jimaek commented 1 year ago

Its also not correct I think, it shows the first few lines twice

# go run main.go http https://www.jsdelivr.com/ --method GET
> NA, US, (WA), Seattle, ASN:6233, xTom
<!DOCTYPE html> <html lang="en"><head><meta charset="utf-8"> <meta content="width=device-width, initial-scale=1" name="viewport"> <meta name="description" content="Optimi
> NA, US, (WA), Seattle, ASN:6233, xTom
Date: Fri, 28 Apr 2023 12:30:21 GMT
Content-Type: text/html; charset=utf-8
Transfer-Encoding: chunked
Connection: close
Cache-Control: public, max-age=300, must-revalidate, stale-while-revalidate=600, stale-if-error=86400
ETag: W/"ae82-pgsQqvPwyIeqFA+gNk2fTj71qs0"
Link: <https://www.jsdelivr.com/>; rel="canonical"
Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
Vary: Accept-Encoding
x-render-origin-server: Render
x-response-time: 18ms
CF-Cache-Status: HIT
Age: 298
Report-To: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=HChal62YGsfMKSwO%2Bhtu41hpzKRXtBdtv15wc82yVVyP3uN6A8hZL1AkJkt7QASWcR9hm420o75%2BUvCjjEyshnHCBTxj8ykSl6cYHfpl65YBPhye8aqI4ZVJfn9JSm3pguM9"}],"group":"cf-nel","max_age":604800}
NEL: {"success_fraction":0,"report_to":"cf-nel","max_age":604800}
X-Content-Type-Options: nosniff
Server: cloudflare
CF-RAY: 7bef50e77fb2c38f-SEA
Content-Encoding: br
alt-svc: h3=":443"; ma=86400, h3-29=":443"; ma=86400
<!DOCTYPE html> 
MartinKolarik commented 1 year ago

I can't reproduce the repeated "> NA, US, (WA), Seattle, ASN:6233, xTom" issue but there needs to be one empty line to separate the headers from the body @didil.

MartinKolarik commented 1 year ago

We discussed this with @jimaek, please update the PR:

  1. http www.jsdelivr.com - HEAD, only status + headers in the output - no change
  2. http www.jsdelivr.com --method GET - GET, only body in the output - no change
  3. http www.jsdelivr.com --method GET --full - GET, status + headers and body in the output, separated with one empty line (the status, e.g., HTTP/1.1 301 is currently missing in this case in this PR)
  4. http www.jsdelivr.com --full, same as 3. because --full automatically implies --method GET

The status + headers always go to stdout as they are only shown when the user explicitly requests them. Only the > NA, US, (CA), San Jose, ASN:397270, NetInformatik Inc. line goes to stderr

Additionally please look into @jimaek's issue with the duplicated line: https://github.com/jsdelivr/globalping-cli/pull/58#issuecomment-1527508758

didil commented 1 year ago

@MartinKolarik a/ for http get --full, in order to write the status in the format HTTP/1.1 301 I would also need the http version, but I can't find that in the api response. b/ when there are multiple probes, before the separator line > NA, US ..., there is an empty line, should I output the empty line to stderr or stdout ?

didil commented 1 year ago

I also can't replicate the duplicate lines issue

jimaek commented 1 year ago

I also can't replicate the duplicate lines issue

Interesting, I couldn't either but I found how:

  1. Run this first go run main.go http https://www.jsdelivr.com --method GET
  2. Run this next go run main.go http https://www.jsdelivr.com/ --method GET

Now the first has no HTML at all, and the second duplicates half the html and metadata

didil commented 1 year ago

@jimaek can you please record a screencast ? I can't see the duplication

jimaek commented 1 year ago

Indeed it doesnt happen every single time. But do you at least see the issue of completely missing HTML?

I caught it on camera only once, in the beginning, you can ignore the rest of the video https://dl.dropboxusercontent.com/s/y73yppnqyi7yyzz/WindowsTerminal_2023-05-01_12-39-32.mp4

didil commented 1 year ago

@jimaek

{ "status": "failed", "resolvedAddress": "78.111.103.62", "headers": { "server": "nginx", "date": "Mon, 01 May 2023 10:40:48 GMT", "content-type": "text/html; charset=utf-8", "transfer-encoding": "chunked", "connection": "close", "vary": "Accept-Encoding, Accept-Encoding", "cf-ray": "7bf17ec31d06b742-AMS", "cache-control": "public, max-age=300, must-revalidate, stale-while-revalidate=600, stale-if-error=86400", "etag": "W/\"ae82-pgsQqvPwyIeqFA+gNk2fTj71qs0\"", "link": "<https://www.jsdelivr.com/>; rel=\"canonical\"", "strict-transport-security": "max-age=31536000; includeSubDomains; preload", "cf-cache-status": "DYNAMIC", "x-render-origin-server": "Render", "x-response-time": "28ms", "set-cookie": [ "__cf_bm=FdOmrLqHM3qkZuOKOXqtpSynwwwT.OAFWzIUlptVaTY-1682707871-0-ASc3+lm3OMmedlRZAZmQlkFCxMrvb1hp/6+t0KnYaEcMADH4l9AhMqrh7GXyQ37hd8apEWJRFG2ePxNf0rZjFrw=; path=/; expires=Fri, 28-Apr-23 19:21:11 GMT; domain=.onrender.com; HttpOnly; Secure; SameSite=None" ], "cache": "HIT, HIT", "x-cached-since": "2023-04-28T18:51:11+00:00, 2023-05-01T10:36:17+00:00", "x-id": "am3-up-gc88, wbs-up-gc4", "x-nginx": "nginx-be, nginx-be", "content-encoding": "br" }, "rawHeaders": "Server: nginx\nDate: Mon, 01 May 2023 10:40:48 GMT\nContent-Type: text/html; charset=utf-8\nTransfer-Encoding: chunked\nConnection: close\nVary: Accept-Encoding\nCF-Ray: 7bf17ec31d06b742-AMS\nCache-Control: public, max-age=300, must-revalidate, stale-while-revalidate=600, stale-if-error=86400\nETag: W/\"ae82-pgsQqvPwyIeqFA+gNk2fTj71qs0\"\nLink: <https://www.jsdelivr.com/>; rel=\"canonical\"\nStrict-Transport-Security: max-age=31536000; includeSubDomains; preload\nVary: Accept-Encoding\nCF-Cache-Status: DYNAMIC\nx-render-origin-server: Render\nx-response-time: 28ms\nSet-Cookie: __cf_bm=FdOmrLqHM3qkZuOKOXqtpSynwwwT.OAFWzIUlptVaTY-1682707871-0-ASc3+lm3OMmedlRZAZmQlkFCxMrvb1hp/6+t0KnYaEcMADH4l9AhMqrh7GXyQ37hd8apEWJRFG2ePxNf0rZjFrw=; path=/; expires=Fri, 28-Apr-23 19:21:11 GMT; domain=.onrender.com; HttpOnly; Secure; SameSite=None\nCache: HIT\nX-Cached-Since: 2023-04-28T18:51:11+00:00\nX-ID: am3-up-gc88\nX-NGINX: nginx-be\nContent-Encoding: br\nCache: HIT\nX-Cached-Since: 2023-05-01T10:36:17+00:00\nX-ID: wbs-up-gc4\nX-NGINX: nginx-be", "rawBody": null, "statusCode": 200, "statusCodeName": "OK", "timings": { "total": 128, "download": 6, "firstByte": 17, "dns": 89, "tls": 13, "tcp": 3 }, "tls": { "authorized": true, "createdAt": "2023-02-18T00:00:00.000Z", "expiresAt": "2024-02-18T23:59:59.000Z", "issuer": { "C": "GB", "ST": "Greater Manchester", "L": "Salford", "O": "Sectigo Limited", "CN": "Sectigo RSA Domain Validation Secure Server CA" }, "subject": { "CN": "jsdelivr.com", "alt": "DNS:jsdelivr.com, DNS:data.jsdelivr.com, DNS:www.jsdelivr.com" } }, "rawOutput": "" }

didil commented 1 year ago
MartinKolarik commented 1 year ago

a/ for http get --full, in order to write the status in the format HTTP/1.1 301 I would also need the http version, but I can't find that in the api response.

I see, I'll open a separate issue for it after updating the API.

b/ when there are multiple probes, before the separator line > NA, US ..., there is an empty line, should I output the empty line to stderr or stdout ?

stdout

MartinKolarik commented 1 year ago

I see, I'll open a separate issue for it after updating the API.

Actually, since we're updating the fields in jsdelivr/globalping#347, here's what you should do:

  1. http www.jsdelivr.com - rawOutput
  2. http www.jsdelivr.com --method GET - rawBody
  3. http www.jsdelivr.com --method GET --full - rawOutput
  4. http www.jsdelivr.com --full, - rawOutput

After the API update, rawOutput will have exactly the format we want for 3/4, and using rawHeaders/rawBody is no longer needed here since we are printing it all to the same stream.

So essentially, this PR gets simplified to:

didil commented 1 year ago

@jimaek the changes are now implemented