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

/api/v0/dns/ returns HTTP 500 instead of 404 when dnslink is not found #2286

Closed
lidel opened this issue Feb 2, 2016 · 5 comments
Closed
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue
Milestone

Comments

@lidel
Copy link
Member

lidel commented Feb 2, 2016

This is a cosmetic issue. 💐

I implemented PoC for ipfs/ipfs-companion#44 and noticed that lookups for FQDNs without dnslink produce HTTP 500:

2016-02-02-223121_822x400_scrot

After ipfs log level namesys debug I see:

21:31:01.708  INFO    namesys: DNSResolver resolving upt.graphiq.com dns.go:54
21:31:01.749 WARNI    namesys: Could not resolve upt.graphiq.com base.go:21
21:31:03.627  INFO    namesys: DNSResolver resolving gscounters.us1.gigya.com dns.go:54
21:31:03.668 WARNI    namesys: Could not resolve gscounters.us1.gigya.com base.go:21
21:31:05.714  INFO    namesys: DNSResolver resolving adx.adnxs.com dns.go:54
21:31:05.761 WARNI    namesys: Could not resolve adx.adnxs.com base.go:21
21:31:13.095  INFO    namesys: DNSResolver resolving perpro17-ew1b.ml314.com dns.go:54
21:31:13.136 WARNI    namesys: Could not resolve perpro17-ew1b.ml314.com base.go:21

It is easy to reproduce via curl:

curl -vq http://localhost:5001/api/v0/dns/1.gravatar.com                                         ~
*   Trying ::1...
* Connected to localhost (::1) port 5001 (#0)
> GET /api/v0/dns/1.gravatar.com HTTP/1.1
> Host: localhost:5001
> User-Agent: curl/7.45.0
> Accept: */*
> 
< HTTP/1.1 500 Internal Server Error
< Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output
< Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output
< Content-Type: application/json
< Trailer: X-Stream-Error
< Transfer-Encoding: chunked
< Date: Tue, 02 Feb 2016 21:57:16 GMT
< Transfer-Encoding: chunked
< 
{"Message":"lookup 1.gravatar.com on 8.8.8.8:53: no such host","Code":0}

My problem: broken semantics. There was no Internal Server Error. Everything worked as expected. 👌

DNS Lookup that does not find dnslink should return a code in lower range, probably the best one being HTTP 404 (Not Found) or 400 (Bad Request).

A similar issue: #1047

@whyrusleeping whyrusleeping added help wanted Seeking public contribution on this issue exp/novice Someone with a little familiarity can pick up labels Aug 19, 2016
@whyrusleeping
Copy link
Member

@lidel is this still an issue?

@whyrusleeping whyrusleeping added this to the Ipfs 0.4.7 milestone Mar 6, 2017
@lidel
Copy link
Member Author

lidel commented Mar 6, 2017

Yes, I use v0.4.6 and still get HTTP/1.1 500 Internal Server Error for curl example above

@whyrusleeping
Copy link
Member

@lidel Thanks! I'll prioritize getting this fixed

@whyrusleeping whyrusleeping modified the milestones: Ipfs 0.4.8, Ipfs 0.4.7 Mar 10, 2017
@whyrusleeping
Copy link
Member

@lgierth do you think you could handle fixing this one?

@ghost ghost self-assigned this Mar 11, 2017
@ghost ghost added the status/in-progress In progress label Mar 11, 2017
@ghost
Copy link

ghost commented Mar 11, 2017

404 will likely not work out. This API is full craze RPC, and 404 simply means "command not found". I gave it a try in #3777 and the result is that the CLI reports "command not found", and it's really tricky to make it work.

You're right that 500 is too vague to make sense of, but we'll likely have to convey additional info not via HTTP statuses, but via the Code field of the response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue
Projects
None yet
Development

No branches or pull requests

2 participants