Skip to content
This repository was archived by the owner on Nov 9, 2023. It is now read-only.

Conversation

@jpuri
Copy link
Contributor

@jpuri jpuri commented Oct 31, 2022

Remove error thrown when response is seen for id which does not have corresponding request.

@Gudahtt : we had discussion about this on the last PR, but as I was playing around with DAPP action replay feature I could replicate this error. Reason I found is - as service worker restart we wait for state in background to be ready to send message to provider to replay actions (there are couple of asynchronous events to be executed for background state to be initialised). While this is being done provider may send send requests already which will also be replayed.

Thus we need to ignore duplicate responses which are possible after replay feature.

@jpuri jpuri requested a review from Gudahtt October 31, 2022 16:13
@jpuri jpuri requested a review from a team as a code owner October 31, 2022 16:13
@Gudahtt
Copy link
Member

Gudahtt commented Oct 31, 2022

Hmm, I'm still struggling to understand this. We aren't queuing up responses, just the requests, so I still don't understand how we'd get multiple responses.

Do you have more information on how to reproduce this?

@Gudahtt
Copy link
Member

Gudahtt commented Nov 1, 2022

OK I think I understand how this could happen now. A request might get sent twice, and get two responses, in this scenario:

  • The background service worker restarts
  • The provider tries to send a message to the background
  • The provider message is captured in the replay queue
  • The provider message is handled by the background service worker after it has started back up, but before it has finished initialization
  • Upon initialization, the background tells the provider that it has finished initialization, so any unhandled responses are replayed.
  • The original request is handled by the background, and a response is sent
  • The second request is handled by the background, and a response is sent

What I was missing is that I hadn't realized there was a state where the service were was still initializing, but it was able to receive requests.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@jpuri jpuri merged commit b0d7842 into main Nov 2, 2022
@jpuri jpuri deleted the response_error branch November 2, 2022 06:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants