wwwouter / typed-knex

A TypeScript wrapper for Knex.js
MIT License
112 stars 13 forks source link

TypeError: Cannot read properties of undefined (reading 'tableName') #42

Closed bschwartz10 closed 2 years ago

bschwartz10 commented 2 years ago

Issue type:

[ ] Question [x] Bug report [ ] Feature request [ ] Documentation issue

Database system/driver:

[ ] Postgres [x] MSSQL [ ] MySQL [ ] MariaDB [ ] SQLite3 [ ] Oracle [ ] Amazon Redshift

typed-knex version:

[ ] latest [ ] @next [x] .4.5.1 (or put your version here)

Error Message

TypeError: Cannot read properties of undefined (reading 'tableName')
      at new TypedQueryBuilder (/Users/User.Name/Documents/code/app-name/server/node_modules/@wwwouter/typed-knex/src/typedKnex.ts:481:54)
      at TypedKnex.query (/Users/User.Name/Documents/code/app-name/server/node_modules/@wwwouter/typed-knex/src/typedKnex.ts:17:16)
      at Object.getFeaturePermissions (/Users/User.Name/Documents/code/app-name/server/.build/repositories/featurePermissionRepository.js:9:56)
      at getFeaturePermissions (/Users/User.Name/Documents/code/app-name/server/.build/services/featurePermissionService.js:30:46)
      at get (/Users/User.Name/Documents/code/app-name/server/.build/controllers/featurePermissionsController.js:13:100)
      at /Users/User.Name/Documents/code/app-name/server/.build/controllers/handler.js:25:28
      at Layer.handle [as handle_request] (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/layer.js:95:5)
      at next (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/route.js:137:13)
      at Route.dispatch (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/route.js:112:3)
      at Layer.handle [as handle_request] (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/layer.js:95:5)
      at /Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/index.js:281:22
      at Function.process_params (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/index.js:335:12)
      at next (/Users/User.Name/Documents/code/app-name/server/.build/node_modules/express/lib/router/index.js:275:10)
      at /Users/User.Name/Documents/code/app-name/server/node_modules/express-jwt/lib/index.js:131:7
      at /Users/User.Name/Documents/code/app-name/server/node_modules/express-jwt/node_modules/async/lib/async.js:52:16
      at Immediate.<anonymous> (/Users/User.Name/Documents/code/app-name/server/node_modules/express-jwt/node_modules/async/lib/async.js:1206:34)
      at processImmediate (node:internal/timers:464:21)

Steps To Reproduce The server consistently crashes after hitting a few different endpoints. My team did some digging and found the issue that we are seeing is coming from the package reflect-metadata

If you execute the statement Reflect.getMetadataKeys(User) it returns with the list of metadata tags that are on the class. (So in this instance the result is [Symbol(table)])

But, if you then run the function Reflect.getMetadata(Symbol(table), tableClass) , the response is undefined.

But of course, this is not consistent with every successful call as the response should look like: { tableName: 'users' }

Package.json

{
    "dependencies": {
        "@wwwouter/typed-knex": "^4.5.1",
        "dayjs": "^1.10.7",
        "express": "^4.17.1",
        "express-jwt": "^6.1.0",
        "http-errors": "^1.8.1",
        "knex": "^0.95.15",
        "mock-knex": "^0.4.10",
        "serverless-http": "^2.7.0",
        "slugify": "^1.6.5",
        "tedious": "^14.0.0",
        "uuid": "^8.3.2"
    },
    "engines": {
        "node": ">=14.0"
    },
    "devDependencies": {
        "@jest/globals": "^27.3.1",
        "@serverless/eslint-config": "^4.0.0",
        "@tsconfig/node14": "^1.0.1",
        "@types/express": "^4.17.13",
        "@types/express-jwt": "^6.0.4",
        "@types/http-errors": "^1.8.1",
        "@types/jest": "^27.0.2",
        "@types/jsonwebtoken": "^8.5.8",
        "@types/mock-knex": "^0.4.3",
        "@types/serverless": "^1.78.40",
        "@types/supertest": "^2.0.11",
        "@types/uuid": "^8.3.3",
        "@typescript-eslint/eslint-plugin": "^5.4.0",
        "@typescript-eslint/parser": "^5.4.0",
        "dotenv": "^16.0.0",
        "eslint": "^8.3.0",
        "eslint-config-airbnb-base": "^15.0.0",
        "eslint-config-airbnb-typescript": "^16.0.0",
        "eslint-config-prettier": "^8.3.0",
        "eslint-plugin-import": "^2.25.3",
        "fishery": "^2.0.0",
        "jest": "^27.3.1",
        "jsonwebtoken": "^8.5.1",
        "prettier": "^2.4.1",
        "serverless": "^2.70.0",
        "serverless-dotenv-plugin": "^3.12.2",
        "serverless-offline": "^8.3.1",
        "serverless-plugin-typescript": "^2.1.0",
        "supertest": "^6.1.6",
        "ts-jest": "^27.0.7",
        "ts-node": "^10.3.1",
        "type-fest": "^2.5.1",
        "typescript": "^4.4.4"
    }
}
bgilman-nyk commented 2 years ago

I just submitted a PR here to fix this issue. There are two things happening here as a result of tree-shaking (due to the serverless dependency as mentioned above)

1) The deep equal of Symbol('table') === Symbol('table') is failing in the reflect-metadata code when calling the function getTableMetadata 2) tableColumns.get(tableClass) returns an undefined object

By taking both tableyMetadataKey and tableColumns and putting them directly on the Class prototype, all instances of the class will be able to use/reference these objects in order to lookup the appropriate table and column names in a query

wwwouter commented 2 years ago

@bgilman-nyk Thanks for the PR! I used it to create a beta version: 4.6.0-beta.0 I'll try this on a few of my own projects to validate and will release the final 4.6.0 next week.

wwwouter commented 2 years ago

This works, and is available in v4.6.0.

@bschwartz10 Can this be closed?

bschwartz10 commented 2 years ago

@wwwouter Good to close ✅