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

Return null on rawIspInfo instead of '' when data not available #646

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zimbres
Copy link

@zimbres zimbres commented Jul 31, 2024

This is to avoid the wrong format of message:

You're testing from: {"processedString":"172.17.0.1 - private IPv4 access","rawIspInfo":""}

@sstidl
Copy link
Collaborator

sstidl commented Jul 31, 2024

This is to avoid the wrong format of message:

You're testing from: {"processedString":"172.17.0.1 - private IPv4 access","rawIspInfo":""}

Which wrong format message are you referring to?

We are planning to release the next version soon and did refactor the same file, please check branch ipinfo_offlinedb

@zimbres
Copy link
Author

zimbres commented Aug 1, 2024

Lets say, using cli, testing against a local server with the change proposed, a far server with default docker image, and a public test

Marcio@Tower  /d/speedtes-cli
$ ./librespeed-cli.exe --local-json server.json --server 2 --telemetry-json telemetry.json 
Using local JSON server list: server.json
Selected server: Far [far]
You're testing from: {"processedString":"10.10.10.1 - private IPv4 access","rawIspInfo":""}
Ping: 144.45 ms Jitter: 1.05 ms
Download rate:  150.07 Mbps     
Upload rate:    41.46 Mbps   
Share your result: http://localhost:8080/results/?id=9

Marcio@Tower  /d/speedtes-cli
$ ./librespeed-cli.exe --local-json server.json --server 1 --telemetry-json telemetry.json
Using local JSON server list: server.json
Selected server: Local [localhost]
You're testing from: 172.17.0.1 - private IPv4 access
Ping: 1.00 ms   Jitter: 0.00 ms
Download rate:  1042.32 Mbps     
Upload rate:    927.02 Mbps    
Share your result: http://localhost:8080/results/?id=10

Marcio@Tower  /d/speedtes-cli
$ ./librespeed-cli.exe
Retrieving server list from https://librespeed.org/backend-servers/servers.php
Selecting the fastest server based on ping
Selected server: Atlanta, United States (Clouvider) [atl.speedtest.clouvider.net]
Sponsored by: Clouvider @ https://www.clouvider.co.uk/
You're testing from: 178.81.81.75 - Unknown ISP
Ping: 139.82 ms Jitter: 1.41 ms
Download rate:  134.39 Mbps     
Upload rate:    16.16 Mbps   

@sstidl
Copy link
Collaborator

sstidl commented Aug 1, 2024

Thank you for the example of usage but I still don't see what the problem is

@sstidl
Copy link
Collaborator

sstidl commented Aug 1, 2024

@adolfintel would this change break anything? The UI seems to handle the empty string correctly. But will it handle a null value as well?

@adolfintel
Copy link
Member

@sstidl I don't understand the use case either. I made it return an empty string instead of null specifically to avoid "accidental" null pointers, can't you just check if the string is empty?

@zimbres
Copy link
Author

zimbres commented Aug 1, 2024

When the test is performed on public librespeed.org, the value is null as well

image

@adolfintel
Copy link
Member

The empty rawIspInfo string was introduced in 2020, not all the public backend servers are ours, clearly this one hasn't been updated in ages.

@zimbres
Copy link
Author

zimbres commented Aug 1, 2024

So the problem might be on cli not able to parse the json correclty when comes "" instead of null?
I could not go deeper in the "go" stuff

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

Successfully merging this pull request may close these issues.

3 participants