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

Response confusing if no/wrong HTTP headers are set #118

Closed
czellweg opened this issue Feb 27, 2015 · 2 comments
Closed

Response confusing if no/wrong HTTP headers are set #118

czellweg opened this issue Feb 27, 2015 · 2 comments
Milestone

Comments

@czellweg
Copy link

Hey Barna

Great, great piece of software - to the point, simple and well-documented. Kudos!

I have noticed that the responses from the server can be a bit confusing/misleading if the 'Content-Type' header is missing. The server responds with 'ok' (HTTP 200) even thought the requested operation was (semantically) not successful.
I must admit that I have not much experience with node.js applications. It's possibly normal or expected to get the request with a 'Content-Type: application/json' header.

Examples below (name of registered package is 'test-bower'):

  1. unregister / remove package without headers
curl -v -X POST -d '{"name":"test-bower"}' http://localhost:5678/removePackage

Response:

ok

Server output:

[bower]  27/2/15 11:51:12   Removed package: undefined

However, the package has not been removed and was still available in the registry. Looking at the source code in lib/server.js:148 (as an example)

    app.post('/removePackage', function(req, res) {
        if(!authenticate(req, res)) {
            return;
        }

        var packages = [ req.body.name ];

        _packageStore.removePackages(packages);

        if(_repoCacheHandler) {
            _removePackagesFromRepoCaches(packages);
        }

        res.send('ok');
    });

even if no header property can be found in the body of the request, the resulting 'undefined' value will be attempted to be removed from the packages internal store.

Only after setting the correct Content-Type header is the package being removed:

curl -v -X POST -H 'Content-Type: application/json' -d '{"name":"test-bower"}' http://localhost:5678/removePackage

Maybe the response codes could be adjusted accordingly if the package cannot be found or if the required body parameters cannot be found? This would make it less confusing and clearer to the user what is going on.

@Hacklone
Copy link
Owner

Hey,
Thanks for recognizing it, I'm planning to fix multiple mistakes in the 1.0 version like this.

@Hacklone Hacklone added this to the 1.0 milestone Mar 29, 2015
@Hacklone
Copy link
Owner

Additional status codes introduced in 1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants