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.

Sign up for GitHub

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

Jump to bottom

Brubeck Client for Browser #244

Merged
merged 164 commits into from
Nov 20, 2021
Merged

Brubeck Client for Browser #244

merged 164 commits into from
Nov 20, 2021

Conversation

timoxley
Copy link
Contributor

@timoxley timoxley commented Oct 11, 2021 •
edited
Loading

Adds support for browser builds of client.

  • Added some additional steps + verification to browser tests. Now uses two clients with different keys and adds grant permissions + assign storage steps.
  • Fixed CORS requests for requests coming from client to storage node.
  • Removed rootDirs from typescript config, was confusing and used inconsistently anyway.
    Now you have to import … from ../../src/file instead of import … from ../../file
  • Made protocol a direct dependency of broker.

Improved bundling process and reduced size

  • Split Logger, Tracker & Network Node so browser build can import node without any tracker dependencies e.g. express
  • Replaced ethers & bip39 (mnemonic) packages in protocol with equivalent individual ethersproject/* packages. This reduces bundle size.
  • Removed double bundling of core-js, just use usage plugin from `@babel/preset-env (See Do we need both of core-js and core-js-pure in the bundle? zloirock/core-js#848 (comment))
  • Updated dev dependencies & unified dependency versions across packages.

Improved bootstrap time: 1m30s to 12s

Also Note

  • Some diff noise due to reverting some merges from main Reverted those reverts.

New Updates

  • Integrated npm workspaces npm workspaces #259
  • Fixed all eslint warnings.
  • Fixed network browser/karma tests.
  • Removed unused client dependencies.
  • Updated client dependencies.
  • Added a fix for resend flakiness.
  • Prevented dist, build, node_modules, etc from being copied into docker image, it installs rebuilds things anyway.
  • Set up client integration tests & cross client tests to run against monorepo brokers/trackers.
  • Configured husky to run eslint before pushing
  • Enabled client export tests in CI.
  • Removed lerna usage everywhere but the show-versions script.

@github-actions github-actions bot added broker Related to Broker Package ci Related to CI configuration client Related to Client Package network Related to Network Package labels Oct 11, 2021
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from 3f33f94 to 3964c44 Compare October 11, 2021 19:02
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from 3964c44 to db2e155 Compare October 11, 2021 19:04
@github-actions github-actions bot added cli-tools Related to CLI Tools Package dev-config protocol Related to Protocol Package test-utils Related to Test Utils Package labels Oct 13, 2021
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch 5 times, most recently from b33bd31 to ca577cb Compare November 18, 2021 04:02
@timoxley
test(client): Use different clients for publishing+subscribing in Sub…
112304e 
…scriberResendsSequential.

Setup:
Publishes 5 messages, waits until they’re stored by polling /last
endpoint.  Stores list of published messages.

Test:
Ask for resend + subscribe
Publish a message. Add this to the list of published messages.
Wait until we get the resent messages + the new message.

After test:
Wait for last message to be stored before proceeding to next test. The
old messages plus the new message now become what we expect to get from
the resend.
Repeat test 4x

What (usually) happens:
First test succeeds
Second test fails
Other tests usually succeed, but occasionally fail.

I suspected this was related to the storage node being unassigned then
reassigned, but I fixed that and I still saw the issue.
Failure only ever occurs in the “after test” phase, when it’s waiting
for the last message to land in storage. Increasing the “wait for
storage” timeout doesn’t help. Curiously, this message will usually
appear in the resent data once the test starts.

I logged when messages were hitting storage and the missing message
wasn’t being stored until after the “wait for last” step times out. If I
increase the timeout, the message takes longer to store.

What’s happening is that the message isn’t being propagated from the
node to storage until the next test starts.

When the client finishes the resend, and sees it has no further
subscriptions, it unsubscribes the node from the stream before the
message was propagated. In the test, the publisher node is the
subscriber so the client is immediately passed the realtime message.

The test is "fixed" by making the publisher and subscriber
different nodes.

Underlying problem is that the node unsubscribes before it propagates
messages.
@timoxley
build: Fix eslint script to fail if eslint fails.
0b7e07f
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from 43b924d to 0b7e07f Compare November 19, 2021 16:50
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from 314d4b1 to b52e64e Compare November 19, 2021 18:37
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from b52e64e to 6b51665 Compare November 19, 2021 18:47
@timoxley timoxley force-pushed the NET-542-brubeck-client-browser branch from a2be0d5 to 70466a9 Compare November 19, 2021 20:45
@timoxley timoxley merged commit 48e165f into main Nov 20, 2021
@timoxley timoxley deleted the NET-542-brubeck-client-browser branch November 20, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teogeb teogeb teogeb left review comments

@harbu harbu Awaiting requested review from harbu

Assignees
No one assigned
Labels
broker Related to Broker Package ci Related to CI configuration cli-tools Related to CLI Tools Package client Related to Client Package cross-client-testing Related to cross-client testing package dev-config docs network Related to Network Package protocol Related to Protocol Package test-utils Related to Test Utils Package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timoxley @teogeb