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

Fix reply timeouts #37

Merged
merged 3 commits into from
Mar 4, 2019
Merged

Fix reply timeouts #37

merged 3 commits into from
Mar 4, 2019

Conversation

dustinconrad
Copy link
Contributor

@dustinconrad dustinconrad commented Feb 17, 2019

This should fix issue #36

@dsrees
Copy link
Owner

dsrees commented Feb 21, 2019

I'm not 100% sure that this is correct. Let me look into this a little further

@dustinconrad
Copy link
Contributor Author

@dsrees is there anything I can do here to get this moving? Right now I have a hack in place to manually catch the timeout event that is being missed buy the library:

channel.on("") {
    channel.off(it.ref)
}

Copy link
Owner

@dsrees dsrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dustinconrad sorry for not being able to get to this sooner. I dug more into this and your solution is correct. I just wanted to double check. Made a small syntax change request and then i can get this merged and released today

@@ -185,7 +185,7 @@ class PhxPush(
mutPayload["status"] = status

refEvent?.let {
val message = PhxMessage(it, "", "", mutPayload)
val message = PhxMessage(it, "", it, mutPayload)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make a change to clean this up a bit and then it should be good to go

refEvent?.let { safeRefEvent ->
    val message = PhxMessage("", "", safeRefEvent, mutPayload)
    this.channel.trigger(message)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it up but opted to use named parameters instead of passing in the empty string for two of them

@dsrees dsrees merged commit d7acb33 into dsrees:master Mar 4, 2019
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

Successfully merging this pull request may close these issues.

2 participants