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

not found should return 404 #20

Open
Nemo64 opened this issue Jun 20, 2018 · 3 comments
Open

not found should return 404 #20

Nemo64 opened this issue Jun 20, 2018 · 3 comments

Comments

@Nemo64
Copy link

Nemo64 commented Jun 20, 2018

When I request http://geoip.nekudo.com/api/127.0.0.1 then I get an error which is correct, but the http status is 200. The http status should be 404.

@nekudo
Copy link
Contributor

nekudo commented Jun 21, 2018

Hi.

This one is a little tricky. While I totally get your point I decided not to implement this. The reason:
The IP address is an optional parameter. This means the URL http://geoip.nekudo.com/api could respond with either 200 or 404 depending on the clients IP. This however would not be correct in the sense of the 404 status as the resource /api is available.

Hopefully this is understandable.

Regrards
Simon

@nekudo nekudo closed this as completed Jun 21, 2018
@Nemo64
Copy link
Author

Nemo64 commented Jun 21, 2018

Well, I kind of get your point but 404 doesn't say that the url is invalid, it says that the resource is unavailable, which it isn't for that person doing the request.

But even if you dislike 404, i think it is closer to correct than 200 Ok.

It's also more annoying while implementing it since you'll need to specially check if there is an error even when the response was a success. So this simple js implementation (which did work with freegeoip.net) is now flawed (pseudo implementation)

fetch('http://geoip.nekudo.com/api')
    .then(response => console.log('success', response))
    .catch(error => console.error('error', error))

I now need something likes this

fetch('http://geoip.nekudo.com/api')
    .then(response => response.type === 'error' ? Promise.reject(response) : response)
    .then(response => console.log('success', response))
    .catch(error => console.error('error', error))

That is neither intuitive nor documented on your start page https://geoip.nekudo.com

There are also some alternatives if you dislike 404, like 400 Bad Request or even 500 would be better imo. I'd even go as far and say 418 is better than 200 because at least it's an error code.

@nekudo
Copy link
Contributor

nekudo commented Jun 21, 2018

Fair point. I'll give this another thought. (As soon as I can spare a few hours)

@nekudo nekudo reopened this Jun 21, 2018
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