protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

Cannot read properties of undefined (reading 'add')

Open zslucky opened this issue 2 years ago • 3 comments

protobuf.js version: 7.2.3

See related code bellow:

https://github.com/protobufjs/protobuf.js/blob/56b1e64979dae757b67a21d326e16acee39f2267/ext/descriptor/index.js#L207-L220

Looks like Line 219 type.oneofsArray size may got 0, so type.oneofsArray[descriptor.field[i].oneofIndex] will return undefined, when I add && type.oneofsArray.lengh > 0 in Line 218, looks like work correctly. I'm not sure what's the correct logic here.

I used this repo behind @grpc/proto-loader, @grpc/grpc-jsand grpc-reflection-js, as I want to use reflection feature in my client.

So pls see my code below:

import * as grpc from '@grpc/grpc-js';
import * as protoLoader from '@grpc/proto-loader';
import * as grpcReflection from 'grpc-reflection-js';

const credentials = grpc.credentials.createInsecure();
const refclient = new grpcReflection.Client('127.0.0.1:9091', credentials);

const root = await refclient.fileContainingSymbol('some.symbol'); // some symbol in 127.0.0.1:9091
const descriptor = root.toDescriptor();
const descObj = protoLoader.loadFileDescriptorSetFromObject(descriptor, {});
const pkgObj = grpc.loadPackageDefinition(descObj);

Pls see tacktrace bellow:

TypeError: Cannot read properties of undefined (reading 'add')
    at Function.fromDescriptor (/Users/sh22888ml/demospace/delivery-svc-template/node_modules/protobufjs/ext/descriptor/index.js:219:66)
    at Function.fromDescriptor (/Users/sh22888ml/demospace/delivery-svc-template/node_modules/protobufjs/ext/descriptor/index.js:92:42)
    at createPackageDefinitionFromDescriptorSet (/Users/sh22888ml/demospace/delivery-svc-template/node_modules/@grpc/proto-loader/src/index.ts:345:60)
    at Object.loadFileDescriptorSetFromObject (/Users/sh22888ml/demospace/delivery-svc-template/node_modules/@grpc/proto-loader/src/index.ts:427:10)
    at Object.<anonymous> (/Users/sh22888ml/demospace/delivery-svc-template/src/modules/grpc-reflection/grpc-reflection.service.ts:15:33)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Promise.all (index 8)
    at GrpcReflectionSvc.onApplicationBootstrap (/Users/sh22888ml/demospace/delivery-svc-template/src/modules/grpc-reflection/grpc-reflection.service.ts:43:16)
    at async Promise.all (index 0)
    at callModuleBootstrapHook (/Users/sh22888ml/demospace/delivery-svc-template/node_modules/@nestjs/core/hooks/on-app-bootstrap.hook.js:43:5)

zslucky avatar Jun 14 '23 08:06 zslucky

I am also getting an error in the descriptor/index.js file as well: https://github.com/protobufjs/protobuf.js/blob/4436cc748c19b88977ab0dc84e59c42339e00520/ext/descriptor/index.js#L488

but for a different method called oneofsArray:

/Users/cfajardo/Downloads/test-grpc/node_modules/protobufjs/ext/descriptor/index.js:489
            if ((descriptor.oneofIndex = this.parent.oneofsArray.indexOf(this.partOf)) < 0) {
                                                                 ^
TypeError: Cannot read properties of undefined (reading 'indexOf')
    at Field.toDescriptor (/Users/cfajardo/Downloads/test-grpc/node_modules/protobufjs/ext/descriptor/index.js:489:66)
    at Root_toDescriptorRecursive (/Users/cfajardo/Downloads/test-grpc/node_modules/protobufjs/ext/descriptor/index.js:142:40)
    at Root_toDescriptorRecursive (/Users/cfajardo/Downloads/test-grpc/node_modules/protobufjs/ext/descriptor/index.js:146:13)
    at Root.toDescriptor (/Users/cfajardo/Downloads/test-grpc/node_modules/protobufjs/ext/descriptor/index.js:121:5)
    at createPackageDefinition (/Users/cfajardo/Downloads/test-grpc/node_modules/@grpc/proto-loader/build/src/index.js:151:33)
    at Object.loadSync (/Users/cfajardo/Downloads/test-grpc/node_modules/@grpc/proto-loader/build/src/index.js:198:12)
    at file:///Users/cfajardo/Downloads/test-grpc/index.mjs:9:39
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)

If I modify the source code and add a check around the existence of oneofsArray then my code works fine I'm likely going to use patch-package to modify the source and add this check, while hopefully the bug fix is made. Going to try and open a PR for oneofsArray check

cliffordfajardo avatar Oct 28 '23 23:10 cliffordfajardo

@zslucky - were you able to resolve your issue?

cliffordfajardo avatar Oct 28 '23 23:10 cliffordfajardo

@cliffordfajardo the same for now

https://github.com/zslucky/protobuf.js/commit/ff3724bc83478a0bb99bd316dd13c775ba11e579#diff-b9977d6fef68d03d3394141bcfa6596d87615270f326629a8f65df7a59fabf18L215-L221

zslucky avatar Oct 29 '23 11:10 zslucky