-1

I want to setup a remote file serving through server, so I code this script:

var http = require('http'),
express = require("express"),
mysql = require('mysql'),
t = false,
request = require('request'),
app = express(),
connection = mysql.createConnection({
  host: 'xxx',
  user: 'xxx',
  password: 'xxx',
  database: 'xxx'
});
app.get('/files/:hash/:title', function(req, res) {
  var hash = req.params.hash;
  connection.query('SELECT * from table WHERE hash="' + hash + '"', function(err, rows) {
    t = rows;
  });
  if (!t) {
    res.writeHead(400, {
      'Content-Type': 'text/plain'
    });
    res.end('token expired');
  } else {
    var lastItem = t.pop(),
    urls = lastItem.url,
    cookies = lastItem.cookies,
    options = {
      url: urls,
      headers: {
        Cookie: cookies
      }
    };
    req.pipe(request(options)).pipe(res);
  }
});
app.get('/*', function(req, res) {
  res.writeHead(500, {
    'Content-Type': 'text/plain'
  });
  res.end('prmission denied');
});
app.listen(8080);
console.log('Server running');

I returns TypeError: Cannot read property 'url' of undefined on first attempt, after refresh page again, it works perfectly, can anyone please check the code where is the issue ?

Mr Boota
  • 42
  • 1
  • 8
  • check `rows.length` – Jthorpe May 30 '17 at 21:15
  • Looks like your typical every day asynchronous logic failure. – Kevin B May 30 '17 at 21:17
  • Possible duplicate of [How do I return the response from an asynchronous call?](https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) – Kevin B May 30 '17 at 21:20
  • that sql code doesn't look very safe. – Kevin B May 30 '17 at 21:21
  • the timeline of events doesn't quite match your code... the first call should be giving you a 400 instead. i suspect somewhere else in your code you're also using `t`, setting it equal to something other than `false`... – Kevin B May 30 '17 at 21:26
  • `rows.length` returns 0 on first attempt – Mr Boota May 30 '17 at 21:30
  • sometime it returns 400 error too – Mr Boota May 30 '17 at 21:31
  • in your current implementation, each request will use the previous request's hash. Not necessarily from the same user. – Kevin B May 30 '17 at 21:32
  • without a previous request, there are no cookies with that hash ...suspect you want the `if (!t) {...}else{...}` in your callback, and use `if(rows.length)` instead of `if(rows){...}` (or equivalently `if(t){...}` in your current code). – Jthorpe May 30 '17 at 21:35
  • @Kevin, pointed that out 15 minutes ago in my (downvoted) post... – Jthorpe May 30 '17 at 21:36
  • BTW, probably returns a 400 error when you wait long enough. "on first attempt" is probably really your first attempt that returned a response. The thing I think you're missing is that the callback in the second argument to `connection.query(...)` is not evaluated until after the `if (!t) {...}else{...}` has already been evaluated. – Jthorpe May 30 '17 at 21:39
  • by using `if(rows.length)` it returns ` if(rows.length){ ^ TypeError: Cannot read property 'length' of undefined` – Mr Boota May 30 '17 at 21:46
  • rows is `undefined` when err is not `undefined`, so be sure and check the value of `err`. Gotta run, so I won't be able to help any more on this today... – Jthorpe May 30 '17 at 22:12

1 Answers1

-1

I suspect it's that this callback:

 connection.query('SELECT * from table WHERE hash="' + hash + '"', 
    function(err, rows) {t = rows; });

is evaluated on the next tick of the event loop, whereas, this line:

urls = lastItem.url, 

is evaluated in the current scope, so you're always returning data from the previous request (which is a strange behavior). On the first query, I'm guessing there are no rows in your table with the current hash, which is why t.pop() is undefined. (note that an empty array is truthy which is why if(t){...} executes even though the number of rows is zero)

Jthorpe
  • 9,756
  • 2
  • 49
  • 64
  • I agree that this is your every day asynchronous logic failure, but why the down vote? – Jthorpe May 30 '17 at 21:20
  • yeah you are right, I'm not familiar with node js logic, I'm newbie, It doesn't return data from SQL, thats glitch, how can I make it working ? – Mr Boota May 30 '17 at 21:29
  • I did, i don't see it as being useful. It simply points out the problem without presenting a solution. though.. i doubt you could provide a solution that hasn't already been provided at the duplicate. – Kevin B May 30 '17 at 21:39
  • Ok, everyone has their own standards, but helping to clarify the source of an error is often quite helpful... – Jthorpe May 30 '17 at 22:11
  • @KevenB BTW, Thanks for the downvote (seriously). It's good to remember that a reference to a well asked and answered SO post is probably better than and incomplete answer. On that note, I think your downvotes would be better received as guidance if you regularly commented (maybe just cut/paste) something like "Downvoted this answer because a link to better answer would been more helpful". – Jthorpe May 31 '17 at 16:17