Nested requests using request-promise: how to do it?
-
Below is a simplified version of my code. The outer request works. The inner one produces an error that a promise was created but not returned. I thought I was returning OK.
const fs = require('fs'); const request = require('request-promise'); let theBaseURL = "https://pharmacyinformatics.local:3000/v1"; function beforeRender(req, res, done) { let wbcPath = '/inpatient/culturesWbcNA/1/' let patientList = [] request({ url: theBaseURL + "/inpatient/patientsNA/" + facilityQ,//ignore this; it is set elsewhere method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }).then(results => { req.data.patients = results; for (let patient in req.data.patients) { patientList.push(req.data.patients[patient].visits[0].p3_csn.trim()); } if (patientList.length === 0) { done(); } let patientStr = ''; req.data.wbc = []; let wbcCount = patientList.length; patientList.forEach((el, done) => { let loopWbcPath = wbcPath + 'lr_csn=' + el; console.log('loopWbcPath', loopWbcPath); request({ url: theBaseURL + loopWbcPath, method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }).then(resWbc => { let wbc = {} wbc.csn = el; wbc.data = bodyWbc.data[0].lab_results; wbcCount -= 1 req.data.wbc[wbc.csn] = wbc.data; if (wbcCount === 0){ done(); } else { return } }).catch(errWbc => { console.log('err', errWbc); }) }) }).catch(function(err) { console.log('err', err); }) }
-
a promise was created but not returned
well that sounds like a warning that everyone gets when mixing callback based code (the beforeRender) with promises.
FYI since jsreport v2 we added support for promises inside the
beforeRender/afterRender
functions, so your code can be refactored to this:const fs = require('fs'); const request = require('request-promise'); let theBaseURL = "https://pharmacyinformatics.local:3000/v1"; async function beforeRender(req, res) { let wbcPath = '/inpatient/culturesWbcNA/1/' let patientList = [] try { const results = await request({ url: theBaseURL + "/inpatient/patientsNA/" + facilityQ,//ignore this; it is set elsewhere method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }) req.data.patients = results; for (let patient in req.data.patients) { patientList.push(req.data.patients[patient].visits[0].p3_csn.trim()); } if (patientList.length === 0) { return } let patientStr = ''; req.data.wbc = []; await Promise.all(patientList.map(async (el) => { let loopWbcPath = wbcPath + 'lr_csn=' + el; console.log('loopWbcPath', loopWbcPath); const resWbc = await request({ url: theBaseURL + loopWbcPath, method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }) let wbc = {} wbc.csn = el; wbc.data = bodyWbc.data[0].lab_results; req.data.wbc[wbc.csn] = wbc.data; }) } catch (e) { console.error('Error', err) } }
however that code contains a variable called
bodyWbc
that i don't see where it is defined, so make sure to review that code
-
So, As I've tried to understand such things as Promise.all and other options for looping promises together, I encountered a very strange bug. At least I think it's a bug in jsreport. The first code below works. The second does not. There's only one line different. Why won't a foreach in there work? I found this as I was trying to reduce my code to the minimum working and then introduce what I need to build the Promise.all. Obviously, I need a foreach or for to build an array of promises. But I can't even get a regular foreach to work. See the arrows below pointing out the change.
const fs = require('fs'); const request = require('request-promise'); let theBaseURL = "https://pharmacyinformatics.local:3000/v1"; function beforeRender(req, res, done) { let wbcPath = '/inpatient/culturesWbcNA/1/' let patientList = [] let facilityQ = 'p3_facility=CVLB'; console.log('options', req.options); if (typeof req.options != 'undefined') { if (typeof req.options.facility != 'undefined') { facilityQ = 'p3_facility=' + req.options.facility; } } request({ url: theBaseURL + "/inpatient/patientsNA/" + facilityQ, method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }).then(results => { console.log('results', results); req.data.patients = results; for (let patient in results) { patientList.push(results[patient].visits[0].p3_csn.trim()); } if (patientList.length === 0) { done(); } let patientStr = ''; req.data.wbc = []; let wbcCount = patientList.length; let ps = [];// array of promises console.log('patientList', patientList); //<<<<<<<<<<<<<<<<<<<<<<<<<This is where I add the change below. return done(); }).catch(function(err) { console.log('err', err); }) }
const fs = require('fs'); const request = require('request-promise'); let theBaseURL = "https://pharmacyinformatics.local:3000/v1"; function beforeRender(req, res, done) { let wbcPath = '/inpatient/culturesWbcNA/1/' let patientList = [] let facilityQ = 'p3_facility=CVLB'; console.log('options', req.options); if (typeof req.options != 'undefined') { if (typeof req.options.facility != 'undefined') { facilityQ = 'p3_facility=' + req.options.facility; } } request({ url: theBaseURL + "/inpatient/patientsNA/" + facilityQ, method: "GET", json: true, strictSSL: false, headers: { 'User-Agent': 'Super Agent/0.0.1', 'content-type': 'application/json' } }).then(results => { console.log('results', results); req.data.patients = results; for (let patient in results) { patientList.push(results[patient].visits[0].p3_csn.trim()); } if (patientList.length === 0) { done(); } let patientStr = ''; req.data.wbc = []; let wbcCount = patientList.length; let ps = [];// array of promises console.log('patientList', patientList); patientList.foreach(el => {console.log('el', el);})//<<<<<<<<<<<<<<<<<<<<<<<<This is the only change. Breaks the code. return done(); }).catch(function(err) { console.log('err', err); }) }
-
BTW, the error is that the request times out. See error message below. It's as if the
return done();
never gets run when there's a foreach in there.
Message:
Timeout during execution of user script logs: +0 Starting rendering request 85 (user: null) +8 Rendering template { name: ActiveAntibioticsHTML, recipe: html, engine: handlebars, preview: true } +11 Data item not defined for this template. +12 Resources not defined for this template. +15 Executing script GetActivePatientsByFacility Error: Timeout during execution of user script at Timeout._onTimeout (C:\Users\constar1\Documents\SutterNow\Express Projects\AdminServer\node_modules\script-manager\lib\manager-servers.js:152:23) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at unrefdHandle (timers.js:520:7) at Timer.processTimers (timers.js:222:12) From previous event: at Reporter.executeScript (C:\Users\constar1\Documents\SutterNow\Express Projects\AdminServer\node_modules\jsreport-core\lib\reporter.js:273:47) at Scripts._executeScript (C:\Users\constar1\Documents\SutterNow\Express Projects\AdminServer\node_modules\jsreport-scripts\lib\scripts.js:88:38)
-
ok interesting, what jsreport version do you have?
are you really sure that the server at
https://pharmacyinformatics.local:3000/v1
is processing the requests? i mean, that you are sure that server is not getting stuck and not responding. i will try to replicate the issue with the same script but replacing the request to external server with a mock
-
I'm using 2.4.0 due to the integration issue with 2.5.1 I noted in another thread. So my instance of jsreport lives inside the same server app that's returning the data. I use WebStorm as my IDE and in the log there, I see that the initial request is performed, which gets the patients. Then follows two Winston logs. The first confirms that the patients were returned. The second is a status 200. So up to that point we're good.
However, I would have expected to see the console.logs I have in the code, namely:
console.log('results', results); console.log('patientList', patientList);
But I don't see these either in the IDE or the 'debug' output in the report builder.
Anyway, in the IDE, it then hangs and I see the error messages generated by jsreports which I pasted in my last message. I'd paste the whole IDE log here, but there's actual patient info, so I can't. I suppose I could scrub it, but I don't think it would add anything to what I've already said.
Immediately after the jsreports errors I do see one more Winston error which says
Error during processing request at https://pharmacyinformatics.local:3000/reporting/api/report/ActiveAntibioticsHTML
That's the report I'm trying to run.
I have about 100 reports that follow a similar pattern to the one I'm trying to get working as my test of jsreports. So if I can just get this one to work, I should have a good pattern to follow and that will allow us to move forward with jsreports rather than having to look elsewhere for a solution. I appreciate how responsive you've been so far with a tester (i.e. non-paying customer yet). Thanks.
-
FYI, this was the thread where I reverted to 2.4.0: https://forum.jsreport.net/topic/1090/integration-into-existing-express-application-error
-
One more note: I see when I remove that one line with the foreach that immediately after the status 200 log message in the IDE that the console.logs from my function show up like they should. So I'd say the hangup is in the beforeRender function.
But I just now saw that you also replied to my first post here. I think you were writing at the same time I was writing my second post here, so I'll go check out what you said and see if I can incorporate that. Looks like you said there is a new promise version of beforeRender instead of the callback version. Is it in 2.4.0? I guess I'll find out in a few minutes. I think I'll go eat lunch first, though.
-
Looks like you said there is a new promise version of beforeRender instead of the callback version. Is it in 2.4.0?
yes, this is supported since 2.0.0
i will try now to replicate your bug in 2.4.0, but i think that bug is not present in 2.5.0. i will confirm this.
FYI, this was the thread where I reverted to 2.4.0
yes, i remember you had problems integrating into express, and we found solution for it that needed that you installed the fix from repository using
npm i jsreport/jsreport-studio
but then you were getting more errors. i did not have the chance yet to check deeper but i'm thinking that the rest of your problems were related to the fact that you are adding jsreport express app after the middlewares of your app and that the jsreport express app works standalone (it has its own set of middleware), i tried to explain it to you here in this comment but probably the idea was not clear. so i will take the chance to show you with the same code that you posted originally.import http from 'http'; import https from 'https'; import fs from 'fs'; import express from 'express'; import session from 'express-session'; import cors from 'cors'; import path from 'path'; import morgan from 'morgan'; import helmet from 'helmet'; import passport from 'passport'; import flash from 'connect-flash'; import cookieParser from 'cookie-parser'; import routes from './routes'; import { seq, Session } from './appDB'; // This only contains the admin tables, not the HL7 stuff import winston from './logging/winstonConfig'; // import Agenda from 'agenda'; // TODO uncomment // import Agendash from 'agendash'; // TODO uncomment // @TODO remove the "force: true" or set it to false for production as this DROPs all tables each time we run the app /** * Do this here so we can sync the WinstonSequelize model too */ console.log('root dir', __dirname); seq.sync({ force: false })// TODO use .then() to log a Winston entry; can't log to DB until DB is ready .catch(function(err) { throw new Error(err); }); import { extDef } from './model/session'; let debug = require('debug')('sql-rest-api:server'); require('./authN/passportConfig')(passport); let SequelizeStore = require('connect-session-sequelize')(session.Store); let app = express(); // const agendaConnectionOpts = {db: {address: 'localhost:27017/agenda', collection: 'agendaJobs', options: { useNewUrlParser: true }}}; // TODO uncomment // const agenda = new Agenda(agendaConnectionOpts); // TODO uncomment //*********** HERE IS THE ONLY CHANGE, BUT THIS CHANGE IS BIG ************** // WE REGISTER THE JSREPORT APP BEFORE ANY OTHER EXPRESS CODE, // IT IS THE FIRST ROUTE DEFINED, AND THERE IS NOT OTHER MIDDLEWARE THAT WILL // BE RUN BEFORE THE jsreport express app, THIS IS IMPORTANT BECAUSE JSREPORT HAS // ITS OWN SET OF MIDDLEWARE AND SINCE NO OTHER MIDDLEWARE // WILL BE RUN BEFORE THIS THEN IT WON'T GET INTO CONFLICTS // Start JSReport const reportingApp = express(); app.use('/reporting', reportingApp); //===================================================================== const corsOptions = { origin: 'https://' + process.env.ORIGIN_DOMAIN, // TODO change this to whatever we use for the live site methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', optionsSuccessStatus: 200, // some legacy browsers (IE11, various SmartTVs) choke on 204 credentials: true // required to send cookies from browsers; didn't need this for postman }; // view engine setup app.set('views', path.join(__dirname, 'views')); app.set('view engine', 'pug'); /** * Get port from environment and store in Express. */ let port = normalizePort(process.env.PORT || '3000');// process.env.PORT is set by iisnode app.set('port', port); /** * Create HTTPS server. */ if (process.env.NODE_ENV !== 'production') { const options = { key: fs.readFileSync('./src/tls/pharmacyinformatics.local.key.pem'), cert: fs.readFileSync('./src/tls/pharmacyinformatics.local.crt.pem') }; app.server = https.createServer(options, app); } else { app.server = http.createServer(app); } /** * Setup morgan format */ morgan.token('id', function (req) { return req.sessionID }); morgan.token('statusMessage', function (req, res) { if (typeof req.flash === 'function' && typeof req.flash('error') !== 'undefined' && req.flash('error').length !== 0) { return req.flash('error').slice(-1)[0]; } else { return res.statusMessage; } }); // This format is for Morgan (HTTP) messages only; see winstonConfig for other messages' format // let morganFormat = ':status :statusMessage - SessionID\: :id :remote-addr ":method :url HTTP/:http-version" ":referrer" ":user-agent" ":res[content-length]"'; let morganFormat2 = '{"status"\:":status","statusMessage"\:":statusMessage","sessionID"\:":id","remote"\:":remote-addr","method"\:":method","url"\:":url","http"\:":http-version","referrer"\:":referrer","agent"\:":user-agent","length"\:":res[content-length]"}'; app.use(express.static(path.join(__dirname, 'public'))); app.use(morgan(morganFormat2, { stream: winston.stream })); app.use(express.json()); app.use(express.urlencoded({ extended: false })); // Middleware app.use(helmet()); // Add Helmet as a middleware app.use(cors(corsOptions)); app.use(cookieParser()); //========================================================================= //---------------------------BEGIN PASSPORT SETUP-------------------------- //========================================================================= // see the express-session and connect-session-sequelize docs for parameter descriptions let sessParams = { secret: process.env.SESSION_SECRET || 'secret',// session secret; TODO replace with environment variable resave: false, saveUninitialized: false,// not sure if this should be true cookie: { domain: 'pharmacyinformatics.local', // TODO change this to whatever we use for the live site sameSite: 'strict', httpOnly: false, // if this isn't set to false, we can't read it via javascript on the client maxAge: 1000*60*20, // 20 minutes path: '/' // / is the root path of the domain }, store: new SequelizeStore({ db: seq, table: 'Session', extendDefaultFields: extDef, checkExpirationInterval: 0 }), rolling: true, // this allows maxAge to be reset with every request, so it moves with activity unset: 'keep' // this is supposed to keep rows in the DB }; if (app.get('env') !== 'production') { // TODO check that we only want this in dev // app.set('trust proxy', 1) // trust first proxy; set this is behind a proxy sessParams.cookie.secure = true // serve secure cookies } app.use(session(sessParams)); app.use(passport.initialize()); app.use(passport.session()); // persistent login sessions app.use(flash()); // use connect-flash for flash messages stored in session //========================================================================= //---------------------------END PASSPORT SETUP---------------------------- //========================================================================= // API routes V1 // app.use('/dash', ensureAuthenticated, Agendash(agenda)); // TODO use this line instead of the next one // app.use('/dash', Agendash(agenda)); // TODO uncomment for test app.use('/v1', routes); /** * error handler * error handling must be placed after all other app.use calls; https://expressjs.com/en/guide/error-handling.html */ app.use(function(err, req, res, next) { let route = ''; // console.log('Called second', err.message); //see catchall for what is called first // won't catch 401 Unauthorized errors. See authenticate.js for 401s if (!err.statusCode) err.statusCode = 500; // Sets a generic server error status code if none is part of the err // let data = JSON.parse(req.user.dataValues.data); // ${data.flash.error[0]} if (typeof err.route !== 'undefined') { route = err.route } else { route = 'Generic' } if (typeof req.user !== 'undefined' && req.user !== null) { if (typeof req.user.dataValues === 'undefined') { winston.error(`${err.statusCode || 500} ${err.message} - SessionID: ${req.sessionID} (${req.user.displayName})\" ${req.ip} \"${req.method} ${req.originalUrl} HTTP/${req.httpVersion}\" \"${req.headers['referer']}\" \"${req.headers['user-agent']}\" \"${req.headers['content-length']}\"`, {username: req.user.sAMAccountName, sessionID: req.sessionID, ip: req.ip, referrer: req.headers['referer'], url: req.originalUrl, query: req.method, route: route}); } else { winston.error(`${err.statusCode || 500} ${err.message} - SessionID: ${req.user.dataValues.sid} (${req.user.dataValues.username})\" ${req.ip} \"${req.method} ${req.originalUrl} HTTP/${req.httpVersion}\" \"${req.headers['referer']}\" \"${req.headers['user-agent']}\" \"${req.headers['content-length']}\"`, {username: req.user.dataValues.username, sessionID: req.user.dataValues.sid, ip: req.ip, referrer: req.headers['referer'], url: req.originalUrl, query: req.method, route: route}); } } else { winston.error(`${err.statusCode || 500} ${err.message} - \" ${req.ip} \"${req.method} ${req.originalUrl} HTTP/${req.httpVersion}\" \"${req.headers['referer']}\" \"${req.headers['user-agent']}\" \"${req.headers['content-length']}\"`, {sessionID: req.sessionID, ip: req.ip, referrer: req.headers['referer'], url: req.originalUrl, query: req.method, route: route}); } if (err.shouldRedirect || err.statusCode === 500) { res.render('error', { error: err }) // Renders a error.html for the user } else { //The below message can be found in the catch's error.response.data item res.status(err.statusCode).send({"customMessage": err.message}); // If shouldRedirect is not defined in our error, sends our original err data } }); // production error handler /*const HTTP_SERVER_ERROR = 500; app.use(function(err, req, res, next) { if (res.headersSent) { console.log('headers already sent'); return next(err); } console.log('handling error'); return res.status(err.status || HTTP_SERVER_ERROR).render('500'); });*/ /** * Listen on provided port, on all network interfaces. */ const server = app.server.listen(port); app.server.on('error', onError); app.server.on('listening', onListening); process.on('unhandledRejection', onUnhandledRejection); process.on('uncaughtException', onUncaughtException); const jsreport = require('jsreport')({ extensions: { express: { app: reportingApp, server: server }, }, appPath: "/reporting", tempDirectory: "temp", configFile: __dirname + "/jsrconfig/jsreport.config.json" }); jsreport.init().then(() => { console.log('jsreport server started') }).catch((e) => { console.error(e); }); // End JSReport async function graceful() { // await agenda.stop(); // TODO uncomment } process.on('SIGINT', graceful); /** * Normalize a port into a number, string, or false. * @param {string} val - The port, input as a string to be parsed as an integer. * @return Return the original val if no integer can be parsed; return integer port number if val was parsable; otherwise return false. */ function normalizePort(val) { let port = parseInt(val, 10); if (isNaN(port)) { // named pipe return val; } if (port >= 0) { // port number return port; } return false; } function onUnhandledRejection(error) { // Will print "unhandledRejection err is not defined" console.log('unhandledRejection', error.message); } function onUncaughtException(error) { console.log('unhandledException', error.message); } /** * Event listener for HTTP server "error" event. * @param {Error} error - An error object. */ function onError(error) { console.log('onError'); if (error.syscall !== 'listen') { throw error; } let bind = typeof port === 'string' ? 'Pipe ' + port : 'Port ' + port; // adding this line to include winston logging //winston.error(`${err.status || 500} - ${err.message} - ${req.originalUrl} - ${req.method} - ${req.ip}`); // handle specific listen errors with friendly messages switch (error.code) { case 'EACCES': console.error(bind + ' requires elevated privileges'); process.exit(1); break; case 'EADDRINUSE': console.error(bind + ' is already in use'); process.exit(1); break; default: throw error; } } /** * Event listener for HTTP server "listening" event. */ function onListening() { let addr = app.server.address(); let bind = typeof addr === 'string' ? 'pipe ' + addr : 'port ' + addr.port; // the following probably won't get to the DB because it isn't online when it is called // winston.info('Listening on ' + bind, {sessionID: 'SYSTEM', username: 'SYSTEM', ip: addr}); console.log('Listening on ' + bind); } export { app, // agenda, jsreport };
to understand what was changed search for the string
HERE IS THE ONLY CHANGE, BUT THIS CHANGE IS BIG
in the above code, if you change your code to this and you have installed the fix we mentioned here i'm almost sure that there should be no other problem.now i'll try to confirm that the bug that you found in scripts is not present in 2.5.0...
-
actually it is not a bug in jsreport, the error is just in your code, here is the problematic line:
patientList.foreach(el => {console.log('el', el);})//<<<<<<<<<<<<<<<<<<<<<<<<This is the only change. Breaks the code.
the right code is
patientList.forEach(el => {console.log('el', el);})//<<<<<<<<<<<<<<<<<<<<<<<<This is the only change. Breaks the code.
it is just a typo
foreach
->forEach
you see the render hanging because that typo, you can make the error more visible if you modify your catch handler to this:
}).catch(function(err) { console.log('err', err); // this makes sure that the script is finished, but with error, and the render does not hang // callback based code is always hard to manage, so if possible try to switch to the `async/await` syntax that i shared with you done(err) })
-
Oh for Pete's sake! Typos!!!
"E"
I should have copy/pasted into my IDE. I'm sure it would have caught it. FYI, I have moved where I had jsreport added to my index file so as to avoid my other middleware per your suggestion. That worked fine. I suppose I should try 2.5 again once I take a break - maybe after I finish this report.
I also began incorporating your suggestion to use async/await and so far that looks good. I have a couple more things to check and then add in, but I think I understand it now. Again, I appreciate your help and patience.
-
Oh for Pete's sake! Typos!!!
"E"
I should have copy/pasted into my IDE. I'm sure it would have caught ithaha that happens to everyone.
I have moved where I had jsreport added to my index file so as to avoid my other middleware per your suggestion. That worked fine. I suppose I should try 2.5 again once I take a break - maybe after I finish this report.
sure, i just wanted to show you that there should be no weird problems when integrating jsreport with express, just relax and when your report is ready you can give jsreport 2.5.0 another try.
I also began incorporating your suggestion to use async/await and so far that looks good. I have a couple more things to check and then add in, but I think I understand it now. Again, I appreciate your help and patience.
awesome, no problem! good luck with that!