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

FileBox toJSON issue #211

Open
hcfw007 opened this issue May 12, 2022 · 6 comments
Open

FileBox toJSON issue #211

hcfw007 opened this issue May 12, 2022 · 6 comments
Labels
question Further information is requested

Comments

@hcfw007
Copy link
Member

hcfw007 commented May 12, 2022

https://github.com/huan/file-box/blob/d40ead1a6f6bd3ff0a2de0fe44ad399784d6a229/src/file-box.ts#L683-773

From above code, we can see that only uuid, url and qrcode fileboxes can call toJson method. However here:

const yellowFileBoxTypes = [
FileBoxType.Buffer,
FileBoxType.Base64,
]
const canPassthrough = (fileBox: FileBoxInterface) => {
/**
* 1. Green types: YES
*/
if (greenFileBoxTypes.includes(fileBox.type)) {
return true
}
/**
* 2. Red types: NO
*/
if (!yellowFileBoxTypes.includes(fileBox.type)) {
return false
}
/**
* 3. Yellow types: CHECK size
*/
const size = fileBox.size
if (size < 0) {
// 1. Size unknown: NO
return false
} else if (size > PASS_THROUGH_THRESHOLD_BYTES) {
// 2. Size: bigger than threshold: NO
return false
} else {
// 3. Size: smaller than threshold: YES
return true
}
}
const normalizeFileBoxUuid = (FileBoxUuid: typeof FileBox) => async (fileBox: FileBoxInterface) => {
if (canPassthrough(fileBox)) {
return fileBox
}
const stream = await fileBox.toStream()
const uuid = await FileBoxUuid
.fromStream(stream, fileBox.name)
.toUuid()
const uuidFileBox = FileBoxUuid.fromUuid(uuid, fileBox.name)
return uuidFileBox
}

We can see that buffer and base64 fileboxes which are less than 20K size will pass. And this will result in error:

18:29:00 ERR PuppetServiceImpl grpcError() roomAvatar() rejection: FileBox.toJSON() can only work on limited FileBoxType(s). See: <https://github.com/huan/file-box/issues/25>
Error: FileBox.toJSON() can only work on limited FileBoxType(s). See: <https://github.com/huan/file-box/issues/25>
    at FileBox.toJSON (C:\juzi\node_modules\file-box\dist\cjs\src\file-box.js:557:23)
    at JSON.stringify (<anonymous>)
    at serializeFileBox (C:\juzi\node_modules\wechaty-puppet-service\dist\cjs\src\server\puppet-implementation.js:70:21)
    at async roomAvatar (C:\juzi\node_modules\wechaty-puppet-service\dist\cjs\src\server\puppet-implementation.js:882:43)
@huan
Copy link
Member

huan commented May 14, 2022

That's by design and is expected behavior.

By our current design, all Base64 / Stream type FileBox must be converted to UUID type before passing it through Puppet Service.

The 20K size limitation is for testing purposes.

@huan huan added the question Further information is requested label May 14, 2022
@hcfw007
Copy link
Member Author

hcfw007 commented May 16, 2022

I didn't get it.
So the design is all Base64 / Stream and Buffer FileBoxes should be converted to UUID, but the actual code allows those types with less than 20K size to pass, right?

@huan
Copy link
Member

huan commented May 16, 2022

the design is all Base64 / Stream and Buffer FileBoxes should be converted to UUID

that’s correct.

The latest version should work as expected after the FileBox code fix. Please feel free to let me know if there has any issues with the FileBox related code.

@hcfw007
Copy link
Member Author

hcfw007 commented May 17, 2022

I see you have fixed Base 64 fileboxes in the latest version, however Buffer fileboxes less than 20K are still unhandled and will result in error.

Error: 2 UNKNOWN: Error: FileBox.toJSON() can only work on limited FileBoxType(s). See: <https://github.com/huan/file-box/issues/25>
    at FileBox.toJSON (C:\Users\NickWang\code\wechaty-dev\wechaty-1.0.x\wechaty-puppet-wxwork\node_modules\file-box\dist\cjs\src\file-box.js:570:23)
    at JSON.stringify (<anonymous>)
    at serializeFileBox (C:\Users\NickWang\code\wechaty-dev\wechaty-1.0.x\docker-project\node_modules\wechaty-puppet-service\dist\cjs\src\server\puppet-implementation.js:70:21)
    at async roomAvatar (C:\Users\NickWang\code\wechaty-dev\wechaty-1.0.x\docker-project\node_modules\wechaty-puppet-service\dist\cjs\src\server\puppet-implementation.js:882:43)
    at Object.callErrorFromStatus (/Users/myonlystarcn/LocalDocuments/code/work/wechaty-1.0/wxwork-test/node_modules/@grpc/grpc-js/src/call.ts:81:24)
    at Object.onReceiveStatus (/Users/myonlystarcn/LocalDocuments/code/work/wechaty-1.0/wxwork-test/node_modules/@grpc/grpc-js/src/client.ts:351:36)
    at Object.onReceiveStatus (/Users/myonlystarcn/LocalDocuments/code/work/wechaty-1.0/wxwork-test/node_modules/@grpc/grpc-js/src/client-interceptors.ts:462:34)
    at Object.onReceiveStatus (/Users/myonlystarcn/LocalDocuments/code/work/wechaty-1.0/wxwork-test/node_modules/@grpc/grpc-js/src/client-interceptors.ts:424:48)
    at /Users/myonlystarcn/LocalDocuments/code/work/wechaty-1.0/wxwork-test/node_modules/@grpc/grpc-js/src/call-stream.ts:330:24

@huan
Copy link
Member

huan commented May 17, 2022

Could you please add a unit test to fail the CI so that we can fix it?

Thank you very much!

@hcfw007
Copy link
Member Author

hcfw007 commented May 19, 2022

So far this messageFile grpc call will work, but it will fall back to messageFileStream when dealing with buffer under 20K size. I don't think this is the expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants