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

Cannot read property '0' of undefined #46

Closed
muka opened this issue Nov 5, 2017 · 5 comments
Closed

Cannot read property '0' of undefined #46

muka opened this issue Nov 5, 2017 · 5 comments

Comments

@muka
Copy link

muka commented Nov 5, 2017

Hi
I have a snippet like this (also using express-promise-router from master)

const router = require('express-promise-router')()
router.get(function(req, res) {
  return foo(req.body).then(bar => res.json(bar))
})
app.use(authstuff())
app.use(`/foo`, router)

Running tests I get

     TypeError: Cannot read property '0' of undefined
      at Route.instanceToWrap.(anonymous function) [as get] (node_modules/express-promise-router/lib/express-promise-router.js:80:87)
      at Function.proto.(anonymous function) (node_modules/express/lib/router/index.js:510:19)
      at Function.instanceToWrap.(anonymous function) [as get] (node_modules/express-promise-router/lib/express-promise-router.js:95:45)
      at Object.module.exports.router (routes/token.js:39:12)
      at Object.<anonymous> (app.js:143:18)
      at require (internal/module.js:11:18)
      at require.connect.then.then (index.js:21:25)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:693:18)
      at Async._drainQueue (node_modules/bluebird/js/release/async.js:133:16)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:143:10)
      at Immediate.Async.drainQueues (node_modules/bluebird/js/release/async.js:17:14)

Maybe I am not using properly or should it be a managed case?

Thanks

@mormahr
Copy link
Member

mormahr commented Nov 5, 2017

Hi, thanks for reporting this issue.

The express.Router documentation defines router.get like this: router.METHOD(path, [callback, ...] callback), so i'd say that calling it without a path is not allowed.
express.Router won't crash on this but won't work as you want it to either. (see snippet below).

const express = require("express")
const app = express()

const router = express.Router()
router.get(function (req, res) {
	res.send("test")
})

app.use("/", router)
app.listen(3000)

(Opening http://localhost:3000/ won't show "test")

Use router.get("/", ...) instead, like this:

const express = require("express")
const app = express()

const router = express.Router()
router.get("/", function (req, res) { // <- you can't omit the path when calling .METHOD
	res.send("test")
})

app.use(router) // <- you can omit the path when calling .use
app.listen(3000)

We try to act as a drop-in replacement and try to stay as close to the original behavior as possible, therefor i might provide a "fix" so we don't crash.

Documentation:
http://expressjs.com/en/4x/api.html#router.METHOD
http://expressjs.com/en/4x/api.html#router.use

If anything is still unclear don't hesitate to ask ☺️

@muka
Copy link
Author

muka commented Nov 6, 2017

Hi,
thank you for clarifying! I fixed the wrong call on my code and works now.

So the bug is not really a bug :)

The crash is useful to detect the wrong usage , eventually the user may get warned of the wrong usage, but is really an edge case

Thanks

@muka muka closed this as completed Nov 6, 2017
@neonexus22
Copy link

neonexus22 commented Apr 15, 2019

const router = require('express-promise-router')()
router.get('some-route', function(req, res) { // add the path to which you want to route as first argument
return foo(req.body).then(bar => res.json(bar))
})
app.use(authstuff())
app.use(/foo, router)

// We get that error typically not because of 'express-promise-router', but others errors like invalid function, spelling mistakes.... etc

@megamit
Copy link

megamit commented Jul 17, 2019

I have encountered this error when trying the documented and valid router.route() usage as shown here
https://expressjs.com/en/4x/api.html#router.route
The following code gives the same error as above:

router.route('campaigns/:id')
    .get(async (req, res, next) => {
       /*   ...    */
    })

if you try to give it a sub route so that promise router doesnt error i.e. .get('/',/async (req, res, next) express throws instead:
Error: Route.get() requires a callback function but got a [object String]
Ideally this should be supported. Could this issue be reopened

@mormahr
Copy link
Member

mormahr commented Aug 2, 2019

@megamit could you open a new issue? I'm not sure if this is related to the current one.

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

4 participants