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

Library is fragile in baseUrl configuration #8

Open
dmtrs opened this issue Jul 15, 2014 · 9 comments
Open

Library is fragile in baseUrl configuration #8

dmtrs opened this issue Jul 15, 2014 · 9 comments
Assignees

Comments

@dmtrs
Copy link

dmtrs commented Jul 15, 2014

var prediction = require('predictionio')({
    key: 'xxx',//add valid key here
    baseUrl: 'http://127.0.0.1:8000/'
});
prediction.users.get('nonexistinguserid', function (err, res) {
        console.log('err', err);
        console.log('res', res);
});

Results in

null
{}

This is because the request is structured as http://127.0.0.1:8000//users/nonexistinguserid.json with double // and the response of the server is

{ 
  //...
  links: {},
  text: 'Your request is not supported.',
  body: {},
  files: {},
  buffered: true,
  headers: 
   { 'content-type': 'text/plain; charset=utf-8',
     'access-control-allow-origin': '',
     connection: 'close',
     'content-length': '30' },
  header: 
   { 'content-type': 'text/plain; charset=utf-8',
     'access-control-allow-origin': '',
     connection: 'close',
     'content-length': '30' },
  statusCode: 404,
  status: 404,
  statusType: 4,
  info: false,
  ok: false,
  redirect: false,
  clientError: true,
  serverError: false,
  error: 
   { [Error: cannot GET //users/14.json?pio_appkey=xxx (404)]
     status: 404,
     method: 'GET',
     path: '//users/14.json?pio_appkey=xxx'
  }
  //...
}
@dmtrs dmtrs changed the title Library is fragile with Library is fragile in baseUrl configuration Jul 15, 2014
@ShaunBaker ShaunBaker self-assigned this Jul 15, 2014
@ShaunBaker
Copy link
Owner

Hey

I will take a look and amend this. In the meantime you could remove the trailing / in the baseUrl to make it happy again.

Thanks

@dmtrs
Copy link
Author

dmtrs commented Jul 15, 2014

I see in the code replication of this._config.baseUrl + '/users.json' there could be a function like this.url('users.json') or even this.url('users') to do the handling. what you think?

@ShaunBaker
Copy link
Owner

Sounds good, would you like to fork and add?

@dmtrs
Copy link
Author

dmtrs commented Jul 15, 2014

@ShaunBaker Currently having a look at the test suite

@dmtrs
Copy link
Author

dmtrs commented Jul 15, 2014

From the test suite I can not see if the superagent is actually mocked. Are the test actually making requests to a localhost prediction.io instance?

@ShaunBaker
Copy link
Owner

@dmtrs Yes the tests run against a local instance. When I originally worked on this I was pressed for time to mock etc!

@dmtrs
Copy link
Author

dmtrs commented Jul 15, 2014

@ShaunBaker Do you have any preferences on a mock library in node? I have used http://sinonjs.org/ in some projects

@ShaunBaker
Copy link
Owner

@dmtrs

Been super busy. Did you get anywhere with Sinon or something similar? Let me know and if not I will make some time to improve it.

Thanks

@dmtrs
Copy link
Author

dmtrs commented Aug 14, 2014

Hey Shaun,

I am sorry but I did not have the time to do so. I would not be able to get
on it till middle of September, unfortunately.

On Thu, Aug 14, 2014 at 1:27 AM, Shaun Baker notifications@github.com
wrote:

@dmtrs https://github.com/dmtrs

Been super busy. Did you get anywhere with Sinon or something similar? Let
me know and if not I will make some time to improve it.

Thanks


Reply to this email directly or view it on GitHub
#8 (comment)
.

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