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

Payload must (?) be valid JSON #18

Closed
steve-baldwin opened this issue May 14, 2019 · 6 comments · Fixed by #21
Closed

Payload must (?) be valid JSON #18

steve-baldwin opened this issue May 14, 2019 · 6 comments · Fixed by #21
Labels
enhancement New feature or request

Comments

@steve-baldwin
Copy link

Hi Andy,

Thanks a lot for your module and the work you've invested in it.

Are there any plans to support notification payloads that are not valid JSON? I was hoping to use it for passing an ID (UUID), but it barfs with:

  pg-listen:notification Received PostgreSQL notification on "b2bc-async": 9a87bf09-9e57-452c-869f-010481781f5b +0ms
Fatal database connection error: SyntaxError: Error parsing PostgreSQL notification payload: Unexpected token a in JSON at position 1
    at JSON.parse (<anonymous>)
    at Client.onNotification (/Users/stbaldwin/dev/node/node_modules/pg-listen/dist/index.js:116:51)
    at Client.emit (events.js:189:13)
    at Connection.<anonymous> (/Users/stbaldwin/dev/node/node_modules/pg/lib/client.js:315:10)
    at Connection.emit (events.js:189:13)
    at Socket.<anonymous> (/Users/stbaldwin/dev/node/node_modules/pg/lib/connection.js:125:12)
    at Socket.emit (events.js:189:13)
    at addChunk (_stream_readable.js:284:12)
    at readableAddChunk (_stream_readable.js:265:11)
    at Socket.Readable.push (_stream_readable.js:220:10)
  pg-listen:connection Closing PostgreSQL notification listener. +1m

I can possibly modify the notifying code to create a JSON payload, but it seems like an 'unnecessary' overhead, and it should be up to the consumer to decide what to do with the payload.

Cheers,

Steve

@Ingramz
Copy link

Ingramz commented May 15, 2019

I have a legacy application where I cannot change the payload format, so I think that makes already two valid use cases for non-JSON payloads.

As a workaround I implemented an option to use "raw" payloads some time ago, but I am unsure if it is implemented up to standards of the author: master...Ingramz:master

@andywer
Copy link
Owner

andywer commented May 15, 2019

Hey guys!

How about a serializer and deserializer option? Default would be JSON.stringify and JSON.parse, but you could pass any arbitrary function. In your use case maybe just the String constructor.

@andywer
Copy link
Owner

andywer commented May 15, 2019

Check out #21. What do you think?

@andywer andywer added the enhancement New feature or request label May 15, 2019
@steve-baldwin
Copy link
Author

Hi Andy. Thanks for responding so quickly. I think that sounds like an awesome idea.

@Ingramz
Copy link

Ingramz commented May 15, 2019

Yeah, looks good to me. The idea of serializer/deserializer suits well the packeted nature of the communication, allowing to return more complex objects as well.

@andywer
Copy link
Owner

andywer commented May 15, 2019

Published as v1.3.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants