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

Problem encoding message when using encodeMessageByTopicName(). #39

Closed
hanaldo1 opened this issue Apr 20, 2020 · 6 comments
Closed

Problem encoding message when using encodeMessageByTopicName(). #39

hanaldo1 opened this issue Apr 20, 2020 · 6 comments

Comments

@hanaldo1
Copy link

Hi,

I'm trying to encode message with avro schema using encodeMessageByTopicName() of this library.
But, when I produce a message to same topic from the second time, it gives me a following error.
(only if trying it only one time or this trying is the first time of several attempts, it doesn't give me a error.)

TypeError: getSchemaByTopicName(...)(...).then is not a function
message:"getSchemaByTopicName(...)(...).then is not a function"
stack: "TypeError: getSchemaByTopicName(...)(...).then is not a function at Object.encodeMessageByTopicName

So, I tried to debug your code and found a code that look like a little bit wrong.

In line 60 of encode-function.js, return just a returned value of cache.getById(), that is parsedSchema, not a Promise in spite of received 'then' in line 70.

 59  if (schemaId) {
 60    return registry.cache.getById(schemaId);
 61  }
 ...
 70  const byTopicName = (registry) => (topic, msg, parseOptions = null) => getSchemaByTopicName(registry)(topic, parseOptions).then(({id, parsedSchema}) => encodeMessage(msg, id)(parsedSchema))

So, there is a way to solve this problem?

when I fixed a code like this, my code've run normally.

return Promise.resolve({ id: schemaId, parsedSchema: registry.cache.getById(schemaId) })
@bencebalogh
Copy link
Owner

Hi
I think you're right. I started fixing this, but since the original inception of this lib a few unexpected requests came so I'll do a major refactor to make fixing this and #36 easier and also adding the features in #28

@hanaldo1
Copy link
Author

Ok, thanks bence 😃

By the way, I saw pull request #31 for issue #28 and it already fix this issue via commit 6f7e5aa!!
So if the pull request is merged, I think this issue would have been solved too.

@dlrudwns135
Copy link

i have same error

@exitsmoker
Copy link

I look forward to merge this without sleeping :)

@bencebalogh
Copy link
Owner

just for visibility: I've opened #42 , I am checking at the moment to make sure nothing it works as expected. It'll fix this issue + make future changes more simple

@bencebalogh
Copy link
Owner

should be fixed in 2.1.0

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