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: added grace time to client timeout #59

Merged
merged 4 commits into from
Jan 29, 2024

Conversation

tegefaulkes
Copy link
Contributor

@tegefaulkes tegefaulkes commented Jan 24, 2024

Description

This PR fixes the following issues and addresses some general code clean up.

Issues Fixed

Tasks

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

This allows for a period of time for the RPCServer to respond if it's handler times out at the same time.
@tegefaulkes tegefaulkes self-assigned this Jan 24, 2024
@tegefaulkes tegefaulkes marked this pull request as draft January 24, 2024 05:25
@ghost
Copy link

ghost commented Jan 24, 2024

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

`testProp` is deprecated and `test.prop` is much cleaner to use anyway.
@tegefaulkes
Copy link
Contributor Author

This should've never been merged into staging

    // Wait for server and client to timeout by checking the flag
    await new Promise<void>((resolve) => {
      const checkFlag = () => {
        if (serverTimedOut && clientTimedOut) resolve();
        else setTimeout(() => checkFlag(), 10);
      };
      checkFlag();
    });

I've removed it along with some other related code.

This was a very odd thing to add to the API. So I've removed it.

Also fixed up some tests relating to that.

[ci skip]
@tegefaulkes tegefaulkes marked this pull request as ready for review January 29, 2024 04:16
@tegefaulkes
Copy link
Contributor Author

Ready for merge if anyone wants to have a look over it.

@tegefaulkes
Copy link
Contributor Author

Most of the general clean up was just syntax and coding style fixes to keep things consistent. I added in some missing type annotation since it's better to be explicit about these things.

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