Make publisher confirms more usable with super streams #276
Conversation
- In order to wait for a publisher confirm reliably using the listener registered against a Client, we need access to the publisherId and connectionId to properly scope a publishingId - PublishConfirmResponse and PublishErrorResponse changed so that publisherId is public (readonly) - Client publish_confirm and publish_error listeners are now passed an extra connectionId argument. Combined with the now visible publisherId, a scoped publishingId can be constructed - SendResult changed to include a reference to the publisher, which allows the caller of SuperStreamPublisher#send() to know which StreamPublisher the message was sent to allowing construction of a scoped publishingId.
| export type FilterFunc = (msg: Message) => string | undefined | ||
| type PublishConfirmCallback = (err: number | null, publishingIds: bigint[]) => void | ||
| export type SendResult = { sent: boolean; publishingId: bigint } | ||
| export type SendResult = { sent: boolean; publishingId: bigint; publisher: Publisher } |
There was a problem hiding this comment.
hello @tunniclm instead of sending the whole publisher can you do it like this:
export type SendResult = { sent: boolean; publishingId: bigint; publisherId: number; connectionId: string }There was a problem hiding this comment.
This should work fine with my current implementation of publish confirmations since the publisherId and connectionId are the only fields I need to construct the extended id.
My original rationale was to make it so you could do the same things with a SuperStreamPublisher that you can do with a normal StreamPublisher by exposing which underlying publisher the sent message was routed to. That said, I didn't have any specific use case in mind beyond my own and looking through the API the only one that comes to mind is an alternative implementation of publish confirmations that uses streamPublisher.on('publish_confirm', ...) instead of using the centralized listener.
All that said, I'm happy to make the suggested change if it's a better fit.
| this.checkMessageSize(publishRequestMessage) | ||
| const sendCycleNeeded = this.add(publishRequestMessage) | ||
| const result = { sent: false, publishingId: publishRequestMessage.publishingId } | ||
| const result = { sent: false, publishingId: publishRequestMessage.publishingId, publisher: this } |
There was a problem hiding this comment.
and here we can put
const result = {
sent: false,
publishingId: publishRequestMessage.publishingId,
publisherId: this.publisherId,
connectionId: this.connection.connectionId,
}can you also check in your example if it works?
Thank you
publisherIdandconnectionIdto properly scope apublishingIdPublishConfirmResponseandPublishErrorResponsechanged so thatpublisherIdis public (readonly)publish_confirmandpublish_errorlisteners are now passed an extraconnectionIdargument. Combined with the now visiblepublisherId, a scopedpublishingIdcan be constructedSendResultchanged to include a reference to thepublisher, which allows the caller ofSuperStreamPublisher#send()to know whichStreamPublisherthe message was sent to allowing construction of a scopedpublishingId.