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

How to detect if connection with child was lost? #58

Open
mariovde opened this issue Sep 2, 2020 · 12 comments · May be fixed by #96
Open

How to detect if connection with child was lost? #58

mariovde opened this issue Sep 2, 2020 · 12 comments · May be fixed by #96

Comments

@mariovde
Copy link

mariovde commented Sep 2, 2020

Hi

I am currently using penpal in a setup at a client but we stumbled upon the following "issue":

When the original page in the iframe - that has the penpal library - is replaced by a page that does not have the library and we want to do a call to the child, we get this:

[Penpal] Parent: Sending getState() call

So Child has a function getState. Parent asks the child to do the getState function. This all works when the child has the library running, but we don't get anything when that initial page is changed to another page.
It just "hangs" on the "Sending call"... No error, no disconnect, nothing.

The code we use is this from the parent to the child. We don't see any Error from the catch.
connection.current.promise .then((child) => { console.log(" Child doGetSTate ", child); return child .getState() .then((state) => console.log("save state to local storage ")) .catch((error) => console.log("an error occured in getState", error)); }) .catch((error) => console.log(" error occured in doGetState() ", error));

What are we doing wrong?
TL;DR: we need to know how to intercept or detect when the connection with the child page is lost, so that we cannot do a call anymore to the child, because it just "hangs": [Penpal] Parent: Sending getState() call

Thanks!

Mario

@Aaronius
Copy link
Owner

Aaronius commented Sep 2, 2020

Hey Mario. The problem here is that the parent is assuming that the child is still working on preparing a response. By default, it waits indefinitely, but you can configure a timeout when the connectToChild is called (see https://github.com/Aaronius/penpal#options). Once you configure a timeout, if the parent doesn't hear a response from the child after the timeout passes, your .catch should be hit.

There may be a better way to handle this in Penpal so that you don't have to wait for the timeout to pass in this particular case. Maybe Penpal could detect when the iframe is navigated from one page to another and if there's no attempt from the child to reconnect it could immediately fail method calls from the parent. Something like that. I need to think about this some more.

@Aaronius
Copy link
Owner

Aaronius commented Sep 3, 2020

Sorry, I misspoke. The timeout configuration option only applies to waiting for the initial connection to be established, so it won't work for your specific situation.

@mariovde, would your ideal solution be that Penpal provides an event that notifies you when there's a disconnection and then a separate event that notifies you when there's a re-connection?

@mariovde
Copy link
Author

mariovde commented Sep 3, 2020

@Aaronius Well, okay, that's why we were waiting so long to see the timeout kick in :)... it never came.
Yes, I think it would be good for everyone to know when the connection was lost and when it would reconnect.
That would be awesome for sure!

@Aaronius
Copy link
Owner

Aaronius commented Sep 3, 2020

Okay, thank you. Providing an event for reconnection will be probably simple enough. Providing an event for "disconnection" might be trickier, but I'll play around with some ideas. Thanks for bringing this up.

@Aaronius
Copy link
Owner

@mariovde I'm curious to know what you'd like this API to look like. I've got some ideas, but I don't really love them.

@mariovde
Copy link
Author

@Aaronius how about adding another config option onConnectionLost : function() ?

const connection = connectToChild({
// The iframe to which a connection should be made
iframe,
// Methods the parent is exposing to the child
methods: {
add(num1, num2) {
return num1 + num2;
},
},
onConnectionLost: function(params);
onConnection: function(params);
});

something like that?

@Aaronius
Copy link
Owner

That's where I was headed. It prompts a few questions:

  • Does onConnection get called on the initial connection and all subsequent connections, or just the subsequent connections?
  • What are the params to onConnection. Maybe the child object?
  • Should the child object provided through connection.promise stop functioning after a reconnection? Its API may be different than the newly-connected child API.
  • If onConnection is called on the initial connection and all subsequent connections, does that make onConnection redundant to connection.promise and should we therefore remove connection.promise?

I'm thinking it might be more appropriate to have onDisconnect (assuming this functionality is feasible) and onReconnect, where onReconnect only gets called on subsequent connections. On a reconnect, a new child object would get passed to the onReconnect callback and the child object provided through connection.promise would stop functioning (an error would be thrown if the parent attempts to call its methods).

I'm not in love with it though.

@yowainwright
Copy link

yowainwright commented Oct 10, 2020

Love PenPal! 🔥

I haven't dug in enough to respond completely coherently so excuse misunderstanding.


I like the idea of isStillConnectedToChild.

Rather than PenPal providing a loss of connection to the child, and then a reconnection to the child, it only confirms it "is Still Connected" connected to the child.


Idea

isStillConnectedToChild() // => boolean
// is still connected to the child (?)
// then do stuff

@Aaronius
Copy link
Owner

Thanks for the feedback @yowainwright!

@mathpaquette
Copy link

just thinking out loud, would it make sense to have a heartbeat between parent and child ? If not received, we can assume they are disconnected. still thinking for better solutions.

@ornakash
Copy link

ornakash commented May 6, 2024

We really like penpal. We'd appreciate a "connection lost" feature.
I think what @mariovde suggested is good, but maybe you can add the same thing to the connectToParent.

const connection = connectToChild({
// The iframe to which a connection should be made
iframe,
// Methods the parent is exposing to the child
methods: {
add(num1, num2) {
return num1 + num2;
},
},
onConnectionLost: function(params);
onConnection: function(params);
});
const connection = connectToParent({
methods: {
add(num1, num2) {
return num1 + num2;
},
},
onConnectionLost: function(params);
onConnection: function(params);
});

Kohart added a commit to Kohart/penpal that referenced this issue Nov 29, 2024
Fixes Aaronius#58

Add connection lost and connection established events to Penpal.

* **Parent Connection**:
  - Add `onConnectionLost` and `onConnection` options to `connectToChild` in `src/parent/connectToChild.ts`.
  - Call `onConnectionLost` when the connection with the child is lost.
  - Call `onConnection` when the connection with the child is established or re-established.

* **Child Connection**:
  - Add `onConnectionLost` and `onConnection` options to `connectToParent` in `src/child/connectToParent.ts`.
  - Call `onConnectionLost` when the connection with the parent is lost.
  - Call `onConnection` when the connection with the parent is established or re-established.

* **Documentation**:
  - Update `README.md` to include `onConnectionLost` and `onConnection` options for `connectToChild`.
  - Update `README.md` to include `onConnectionLost` and `onConnection` options for `connectToParent`.

* **Tests**:
  - Add tests in `test/connectionManagement.spec.js` to verify `onConnectionLost` and `onConnection` functionality for `connectToChild`.
  - Add tests in `test/connectionManagement.spec.js` to verify `onConnectionLost` and `onConnection` functionality for `connectToParent`.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/Aaronius/penpal/issues/58?shareId=XXXX-XXXX-XXXX-XXXX).
@Kohart Kohart linked a pull request Nov 29, 2024 that will close this issue
@Aaronius
Copy link
Owner

Aaronius commented Dec 5, 2024

Hey folks, @Kohart wrote up a PR (thank you!) for addressing this issue, but before proceeding with the PR I want to make sure we have a clear and reasonable definition of "connection lost". Here are some example scenarios that may or may not be considered as "connection lost":

  • When the iframe is removed?
  • When the iframe is navigated away from its current document?
  • When a method call doesn't receive a response within a particular time period?
  • When a heartbeat (which doesn't exist yet) fails?
  • When connection.destroy() is called?

To answer this, I think it would be useful to understand how you would plan to use an onConnectionLost feature it in your applications. In other words, if you were notified that the connection was lost, what action would you take within your application?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants