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

Accessing req.routeConfig in frameworkErrors handler throws TypeError #4807

Closed
2 tasks done
sergburn opened this issue Jun 9, 2023 · 11 comments · Fixed by #4826
Closed
2 tasks done

Accessing req.routeConfig in frameworkErrors handler throws TypeError #4807

sergburn opened this issue Jun 9, 2023 · 11 comments · Fixed by #4826
Labels
bug Confirmed bug good first issue Good for newcomers

Comments

@sergburn
Copy link
Contributor

sergburn commented Jun 9, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.17.0

Plugin version

No response

Node.js version

16.x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

11.7

Description

An attempt to get a value of request.routeConfig in frameworkErrors handler throws an exception:

test/router-options.test.js 2>       return this[kRouteContext][kPublicRouteContext].config
test/router-options.test.js 2>                                                       ^
test/router-options.test.js 2> TypeError: Cannot read properties of undefined (reading 'config')

Steps to Reproduce

test('Should not throw on access to routeConfig frameworkErrors handler - FST_ERR_BAD_URL', t => {
  t.plan(6)

  const fastify = Fastify({
    frameworkErrors: function (err, req, res) {
      t.ok(typeof req.id === 'string')
      t.ok(req.raw instanceof Readable)
      t.same(req.routerPath, undefined)
      t.same(req.routeConfig, undefined)
      //         ^^^
      // test/router-options.test.js 2>       return this[kRouteContext][kPublicRouteContext].config
      // test/router-options.test.js 2>                                                       ^
      // test/router-options.test.js 2> TypeError: Cannot read properties of undefined (reading 'config')
      res.send(`${err.message} - ${err.code}`)
    }
  })

  fastify.get('/test/:id', (req, res) => {
    res.send('{ hello: \'world\' }')
  })

  fastify.inject(
    {
      method: 'GET',
      url: '/test/%world'
    },
    (err, res) => {
      t.error(err)
      t.equal(res.body, '\'/test/%world\' is not a valid url component - FST_ERR_BAD_URL')
    }
  )
})

Expected Behavior

Should return undefined as, for example, req.routerPath does

@sergburn
Copy link
Contributor Author

sergburn commented Jun 9, 2023

If this is accepted as a valid issue, which requires a fix, I can work on a pull request.

@gurgunday
Copy link
Member

Yeah I think the getter should return undefined instead of throwing

@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers labels Jun 13, 2023
@ShivangRawat30
Copy link

can I work on this issue ??

@metcoder95
Copy link
Member

Go for it 👍

@ShivangRawat30
Copy link

ShivangRawat30 commented Jun 14, 2023

I looked at the code and figured that if either kRouteContext or kPublicRouteContext is undefined this will result in a TypeError at runtime, the req.routerPath is returning undefined
routerPath: {
get () {
return this[kRouteContext].config.url
}
},
If for routerPath it is returning undefined, this[kRouteContext] might be undefined as well, and that's why this[kRouteContext][kPublicRouteContext].config is throwing as TypeError,
or the problem might be in kPublicRouteContext.
I am new to contributing to open source, can you help me with this ?

@metcoder95
Copy link
Member

If for routerPath it is returning undefined, this[kRouteContext] might be undefined as well, and that's why this[kRouteContext][kPublicRouteContext].config is throwing as TypeError,
or the problem might be in kPublicRouteContext.

It should be a matter of treating with care the returning statement.
e.g. you can use the optional chaining to return undefined if a given value is not present

console.log(this[kRouteContext]?[kPublicRouteContext]); // Will return undefined if `kRouteContext` is undefined

@ShivangRawat30
Copy link

@sergburn I am having hard time Reproducing the Error can you please tell me where you wrote the above code.

@sergburn
Copy link
Contributor Author

@ShivangRawat30 put it to tests/router_options.test.js and add const { Readable } = require('stream') at the top

@ShivangRawat30
Copy link

ShivangRawat30 commented Jun 15, 2023

@metcoder95

console.log(this[kRouteContext][kPublicRouteContext])  //is undefined but when i logged 
console.log(this[kRouteContext]) //it is logging [object, Object]

which means an object is present in this[kRouteContext] but is unable to get the value from [kPublicRouteContext]

@giuliowaitforitdavide
Copy link
Contributor

I provided a fix for this bug in #4826
Sorry @ShivangRawat30 , I didn't read you were already working on that

@metcoder95
Copy link
Member

@metcoder95

console.log(this[kRouteContext][kPublicRouteContext])  //is undefined but when i logged 
console.log(this[kRouteContext]) //it is logging [object, Object]

which means an object is present in this[kRouteContext] but is unable to get the value from [kPublicRouteContext]

That means that kRouteContext does not contains any value, for instance when trying to access config prop it will throw as we are trying to access a nonexistent value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants