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

Send frame should suspend until delivery of the message #125

Closed
4 of 8 tasks
Shusek opened this issue Jun 29, 2021 · 5 comments
Closed
4 of 8 tasks

Send frame should suspend until delivery of the message #125

Shusek opened this issue Jun 29, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@Shusek
Copy link

Shusek commented Jun 29, 2021

Describe the bug

During the tests, it turned out that the sending frame block is not suspending as it would seem logical. I expected until the message not to be sent convertAndSend function suspend my block, instead of that this function returns almost immediately, is a bit strange because the message has not been sent yet. Is this planned behavior?

Reproduction and additional details

  1. Connect to websocket
  2. Turn on airplane mode (or throttle connection)
  3. Below code should raise TimeoutCancellationException or some other error related to connection problem. Instead of expected error i get instant "Sent message to $destination".
    try {
            withTimeout(timeout) {
                getOrConnectSession().convertAndSend(destination, body, serializer)
                Timber.d("Sent message to $destination")

            }
        } catch (e: TimeoutCancellationException) {
            Timber.e(e)
        }
        

Context

Kotlin version:
Krossbow version:
Kotlin target(s): (e.g. browser, NodeJS, JVM, Android, a multiplatform combination?) Android
Artifacts used:

  • krossbow-stomp-core
  • krossbow-stomp-jackson
  • krossbow-stomp-kxserialization
  • krossbow-websocket-core
  • krossbow-websocket-sockjs
  • krossbow-websocket-spring
  • krossbow-websocket-okhttp
  • krossbow-websocket-ktor
@Shusek Shusek added the bug Something isn't working label Jun 29, 2021
@joffrey-bion
Copy link
Owner

joffrey-bion commented Jun 29, 2021

Hi @Shusek, thanks for reporting this!

The expected behaviour is described in the documentation of StompSession:

If no receipt header is provided and auto-receipt is disabled, the method used to send the frame doesn't wait for a
RECEIPT frame and never throws LostReceiptException.
Instead, it returns immediately after the underlying web socket implementation is done sending the frame.

I'm assuming you have autoReceipt disabled and you are not sending any receipt header manually, right? In that case, the suspension behaviour of convertAndSend will be determined by the underlying websocket implementation (the wording of the doc may be misleading here, sorry).

Since you're using Ktor, the suspension will be determined by Ktor's implementation. It seems that there is some buffer in the outgoing frames channel of Ktor's websocket (from Ktor's doc):

Outgoing frames channel. It could have limited capacity so sending too much frames may lead to suspension at corresponding send invocations. It also may suspend if a peer doesn't read frames for some reason.

I guess this is why you observe this behaviour. Most likely if you send a bunch of frames, the convertAndSend calls will eventually suspend.

Could you please explain the use case a bit more in detail? If what you want is to detect when the server has received the message, it would be more correct to enable STOMP receipts instead. Otherwise, why do you need to suspend until the frame is sent but not necessarily until the server has received the message?

@Shusek
Copy link
Author

Shusek commented Jul 8, 2021

My usecase was to check if the connection was broken, but after consideration i can just observe flow and catch exception and retry last message.

@Shusek Shusek closed this as completed Jul 8, 2021
@joffrey-bion
Copy link
Owner

joffrey-bion commented Jul 9, 2021

My usecase was to check if the connection was broken

Mmmh that's actually interesting, as a user I would also expect to get an exception at the moment I call convertAndSend in that case.
A workaround at the moment would be to enable receipts to verify the server gets the messages. In that case convertAndSend would suspend until the server confirms the reception by sending a RECEIPT frame. If the server doesn't get the message (doesn't respond with a RECEIPT in the configured timeout), you get a LostReceiptException on convertAndSend.

after consideration i can just observe flow and catch exception and retry last message

Could you please clarify which flow you're talking about here?
Is it the flow of some STOMP subscription?

@Shusek
Copy link
Author

Shusek commented Jul 9, 2021

As you probably noticed from my previous issue the system I work with is in obsolete phase and that's the main reason why i can't use RECEIPT but I agree that that would be the best option.

At this moment i implemented something close to
#102
Unfortunately, it won't be of any use to anyone else because it is very related to sending messages.
For my business logic, I just need to rely on subscription exception instead expect send to give me some error. If there is an error in subscription flow then resend data on subscribe.
Very simplified pseudocode:
getOrConnectSession() .subscribe("/destination") .onStart{ getOrConnectSession().send(data) } .retry{ true }

@joffrey-bion
Copy link
Owner

@Shusek I'm so sorry I realized I never replied to your last comment. The last snippet you provided shouldn't be sufficient IMO, because you will be missing the actual SUBSCRIBE frame in the new session on retry.

The goal of #102 is not only to reconnect but also to automatically re-send the SUBSCRIBE frame under the new session and start sending the MESSAGE frames from the new subscription into the same flow as before, so that it's transparent to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants