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

Sails responses not working with --no-frontend flag #3557

Closed
revic1993 opened this issue Feb 9, 2016 · 8 comments
Closed

Sails responses not working with --no-frontend flag #3557

revic1993 opened this issue Feb 9, 2016 · 8 comments

Comments

@revic1993
Copy link

When I try to test the controllers using mocha I am getting an empty response body.
EDIT :

module.exports = function(type,data){
  var res = this.res;
  var res_type = sails.config.constants.RESPONSE_TYPE;

  switch (type) {
    case res_type.BAD_REQUEST: res.status(400);
                               break;
    case res_type.FORBIDDEN: res.status(403);
                             break;
    case res_type.OK : res.status(200);
                       break;
    default:res.status(400);
  }

  return res.jsonx(data);
}

I have created this custom response which is working fine. But I still haven't figured out why the inbuilt responses are not working.

@sgress454
Copy link
Member

Hi @revic1993. Sorry you're having a problem with your Sails app. We'll need a bit more information before looking into this (in the future, before posting an issue please take a look at our issue posting guidelines).

  • What version of Sails are you using?
  • What version of Node are you using?
  • What version of NPM are you using?

Also, can you set up a repo that reproduces this issue? For example:

sails new sails-issue-3557 --no-frontend
cd sails-issue-3577
# Now make whichever are the simplest/quickest changes which reproduce the issue,
# for example the test where you're seeing an empty response,
# and when you for sure have the issue isolated, run:
git init
hub create  # or use the GitHub UI to create the repo and `git remote add` it manually
git add .
git commit -am 'Reproduces https://github.com/balderdashy/sails/issues/3557'
git push -u origin master
hub browse # or open the repo in your browser manually
# Then copy the URL of the new repo and paste it as a comment on this issue.
# We'll check it out, and since this way we'll be able to experience the problem
# ourselves, we should have no problem getting to the bottom of it.

@mikermcneil
Copy link
Member

@revic1993 hmm... this might have something to do with the environment configuration your tests are using. Best thing would be for us to take a look at a repo that reproduces this like @sgress454 mentioned. Thanks!

@mikermcneil
Copy link
Member

On a side note, we experimented with this and I think it's time we revert it. res.badRequest(), res.forbidden(), and res.notFound() (everything except res.negotiate() and res.serverError() realistically) are designed to be used for intentional API responses with specific response data. So the response body should not be stripped in production by default (whether or not config is available).

So @revic1993 if it turns out that's your issue, good news is it's really easy to resolve by changing the default files in api/responses/.

@sgress454
Copy link
Member

@mikermcneil Not sure what reverting would do--the default was always to strip the errors in production. That config option was added to allow them to not be stripped.

Unless you mean reverrrrrting, which I'm ambivalent about. The most important thing is it gets logged no matter what--so in this case, there should at least be something in the console...

@mikermcneil
Copy link
Member

@sgress454 it used to only be for res.serverError(), which was the intended behavior. Apparently the way it got in there originally was that I accidentally copied and pasted it everywhere when I was updating the response templates in the backend generator. Then in an effort to avoid changing those files constantly, I left them as is. But realistically it's not a good default, and I'd rather change it now than waiting for 1.0 (because it's in a generator, it won't affect existing apps).

@revic1993
Copy link
Author

Hey guys,
Sorry for late response.
@sgress454 Version info that you asked for:

sails v0.11
npm v2.14.7
node v4.2.2

Here's link to the repo:
https://github.com/revic1993/sails-issue-3557

The res.body is an empty object.

@sgress454
Copy link
Member

@revic1993 Thanks! I cloned your repo and was able to run your test with no issues. I put a console.log(res.body) in the test and got:

{ error: true,
  message: 'You are not allowed to access this url.' }

Following what @mikermcneil said, I'd check that you're not somehow setting your environment to "production". In your test, try console.log(sails.config.environment) and make sure it says "development".

@revic1993
Copy link
Author

@sgress454 Thanks! I got it..
So my environment was production and when I set it to development it is working. So I guess this was causing the data to be undefined :


  if (sails.config.environment === 'production') {
    data = undefined;
  }

I guess I should go with my custom response as I have to use api in the production.
Thanks @sgress454 and @mikermcneil for your help. Appreciate it.

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

No branches or pull requests

3 participants