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

Use HTTP to Check whether Server is Available #52

Open
4 of 15 tasks
Firionus opened this issue May 14, 2021 · 0 comments
Open
4 of 15 tasks

Use HTTP to Check whether Server is Available #52

Firionus opened this issue May 14, 2021 · 0 comments
Assignees
Labels

Comments

@Firionus
Copy link
Contributor

Firionus commented May 14, 2021

Currently (feature/patch-ui ff0bbcac), we use reconnecting-websocket to establish a robust WebSocket connection to the Server. However, Firefox has a very strict rate limiter for failed WebSocket connections (https://bugzilla.mozilla.org/show_bug.cgi?id=711793#c0). In the extreme case, we can only check for a WebSocket connection every 60 seconds. So if we try to reconnect every second on Firefox, this is the resulting network traffic:
firefox_reconnecting_websocket_wireshark
The only way to stop this rate limiting is to restart Firefox. Reload does not work. For comparison, Chrome's rate limiting is much less aggressive:
reconnecting_websocket_chrome

However, this means that if we want to support Firefox properly, we need to use a different mechanism than WebSocket connects for checking whether the server is available.

TODO

  • Research how to find out with HTTP whether a server is reachable
    • new API endpoint?
    • or just make a HTTP request to ws?
    • => make a HEAD request to the server. Could only fail if we hit a cache, but I think our server currently forbids browser-side caching.
  • Implement a ConnectionProvider that handles this (class or hook?)
  • check in regular intervals with HTTP whether the server is available
  • if yes, connect WebSocket once or twice
  • if that fails, go back to checking with HTTP
  • communicate the connection state outwards
    • Subscriptions need to be reset when closing
    • Subscriptions need to be made when opening
    • Connection Error Alert needs to be shown when HTTP cannot reach server
    • Connection Error Alert should communicate when the connection is checked (bar going down, countdown, ?)
    • test with Cypress
    • ensure this also works with the frontend dev server
@Firionus Firionus added the webui label May 14, 2021
@Firionus Firionus self-assigned this May 14, 2021
@Firionus Firionus mentioned this issue May 15, 2021
66 tasks
Firionus added a commit that referenced this issue May 25, 2021
Instead, we'll try once to connectr the WebSocket on page load.
See #52 for
implementing automatic reconnection
Firionus added a commit that referenced this issue Oct 19, 2021
* Add Navbars and Tabs for Patch

Also includes first version of support for hotkeys
to navigate.

* Add Navbars and Tabs for Patch

Also includes first version of support for hotkeys
to navigate.

* Add ag-grid example table to Patch

* Add "Add New Fixtures" Button

* Add example data

Just noticed ag-grid won't support range
selection in community edition. Need to use
different table library, e. g. react-data-grid,
tabulator or react-table with our own frontend.

* React Table Example

* Tabulator Example

* Add Example Data and fix Table Height

* Implement PatchState as Context with UUIDs

* Rudimentary Fixture Type Page

* Add New Fixture Route and complicate Routing

* Rudimentary "Add Fixture" Page Layout

* Add WS connect

Reconnecting WebSocket will try to connect.
If it can't, a blocking pop-up will be displayed.

WS uses Proxy in Development, so that on can use the CRA
development server to handle website request while
cueglow-server handles all WebSocket connections.

* Update dependencies and fix some warnings

* Add Cypress

Includes setup according to docs and removing some warnings

* Add custom npm commands for Cypress

For development, start the dev server with `npm start`, then start the
Cypress Test Runner with `npm run cy`. To connect the WebSocket,
start the cueglow-server from its directory with `gradlew run`. Cypress
will direct its request to locahost:3000.

To test against the production server with Cypress, start cueglow-server
with `gradlew run` and start Cypress with `npm run e2e`. It will then direct
all request to localhost:7000.

* Add Cypress Navigation Test and disable examples

* Set up simple smoke test in Jest

Also refactored index.tsx into App.tsx and index.tsx. Jest renders the
App with some fiddling (not transforming blueprintjs and mocking
out sass variable loader). However, for testing the frontend Cypress
should be preferred. Jest would be useful for simple unit tests that
do not depend on much else in the App, like testing an algorithm.

* Make outline for WS API Mock Test

* Remove Non-API WebSocket Message on Connect

* Update Dependencies

* Remove sass-extract-loader for Blueprint Variables

sass-extract-loader is practically unmaintained, the last commit was
about three years ago. I replaced it with manually added SCSS module
exports that are wrapped in a TypeScript module.

* Update to npm v7

* Replace react-hotkeys-hook with Blueprint

* Remove reconnecting-websocket

Instead, we'll try once to connectr the WebSocket on page load.
See #52 for
implementing automatic reconnection

* Only Render App When Connection is Open

if connection isn't open, we render other pages.

* Refactor App into Multiple Files

* Delete index.css and move to SCSS

* Resolve Lint Warnings

* Clean Up ConnectionProvider Architecture

Specifically, use a Property Event Handler in a connecitonProvider class.
The interface hook was moved to the single component where
it is allowed to be used. The hook registers its own event handler.

* Write Some Types for Patch Messages

* Add Gradle Task without Frontend Build

closes #54

* Subscribe to Patch Topic

Current design is clunky but will be improved down
the line.

* Implement Upload Fixture Type

* Handle Add Fixture Types

* Avoid Setting Patch Callback on Every Change

* Make Patch Data Handler Global Singleton

This way, the message handler can call into the
patch data handler, making it superfluous to register
callbacks.

* Create Classes for Outgoing Messages

* Implement Crude Remove Fixture Types

The UI, the code, everything is suboptimal here.
Significant issues were marked with TODO comments.

* Implement addFixtures and removeFixtures

* Test Add/Remove Fixtures/Fixture Types

Still need to fix warning about Cypress commands in async function.

* Refactor Setup from JS to Cypress Promise

* Refactor Cypress Tests

- Change all files to TypeScript
- Refactor clearFixtureTypes to custom command
- Add dataCy command example from website

* Warn on Explicit any type

* Show Message When No Fixture Type Is Selected

* Change Title and favicon

The green play button isn't final, but better than the React logo.

* Test WebSocket Connection Error Behavior

* Remove Explicit Any

There seems to be issues with typings for react-tabulator not being
recognized.

* Remove TODOs: Refactor PatchDataProvider

* Move Tabulator CSS Imports into index.scss

* Remove TODOs in NewFixture

- missing default handling moved to #50
- current implementation with Blueprint's Suggest works fine
- Triangle is not super needed, so let's drop it

* Wrap ReactTabulator in Strongly Typed Element

We achieve this by using types from @types/tabulator-tables and
applying them for the React element from
react-tabulator.

This has the benefit of much improved IDE tooling and better
type safety.

* Fix: Show Selection Change in FixturePatch

Previously, one had to click twice on a fixture row to
show the selection, although it was selected for remove after the
first click. Fixed with useMemo, though I don't know why it works.

* Allow Selection of Multiple PatchFixtures

Also change selection callbacks from rowSelected to rowSelectionChanged
to include deselection.

* Fix Gradle Deprecation Warnings

* Fix RunNoNpm

Previously, RunNoNpm would exclude the relevant task during the
configuration phase, meaning the excluded task would never run at all.
This is now fixed: npm_run_build runs when using gradle run, but not
with gradle runNoNpm.

* Update Gradle from 6.7 to 7.1

This allows us to use Java 16 (not possible with 6.x due to
gradle/gradle#15538).

* Fix Cypress Jank

* Add Hotkey to Remove Buttons

* Make FID Editable with Validation

Edits aren't sent to the server yet.

* Make Universe and Address in Patch Editable

* Synchronize Patch Fixture Edits to Server

I introduced lodash as depedency. I don't like it, but it's a common
tool and writing the object merge in TypeScript is very annoying.

Open Issue: Empty Values for the numbers either do nothing
or crash the server connection.

* Fix Connection Drop When Entering Empty FID Value

The protocol does not have a reserved missing value for FID, therefore
an empty FID is just not allowed. This is now enforced by validation.

* Refactor Error Message From Tabulator Validators

* Allow Empty DMX Address

## What API allows:
- set dmxAddress to -1 to unpatch
- set universe to -1 to unpatch

## What we do:
- if  universe is -1, we show an empty string with a formatter, but
this can't be set by the user due to
olifolkerd/tabulator#1700
- dmxAddress can be set to empty string which will be translated with a mutator
to -1 for the server and the other way around

* Add Tests for Patch Fixture Editing

* Add Navigation and Add Hotkeys

* Refactor into HotkeyHint Component

* Fix Typo in Patch Tab "FixtureTypes"

added Space between words

* Organize PatchWindow Imports

* Fix Unique Key Warning on HotkeyHint

* Fix Unique Key Warning on HotkeyHint with Chords

Doesn't occur in our application yet, so I missed it in the
previous commit.

* Keyboard-Only Story for Add Fixtures

- proper tab order
- start focus in first field
- advance to next field after hitting enter in suggest fields
- submit on ctrl+enter
- Test for the above

* Fix Crash: Add Fixtures without Selecting GDTF

now form will not submit when no fixture type is selected

* Reset Selected DmxMode when Fixture Type changes

* Declare Min/Max Values on All New Fixture Fields

* Accept -1 for Universe and Address Server-Side

* Half-Working Form Validation with react-hook-form

* Allow Exiting out of NewFixtures with Esc

Normal HTML inputs don't blur on Esc, so it should be okay
to exit the page on Esc as announced by the Hotkey Label.

Possible Issue: A user assumes Esc will exit out of an input and
the user loses the data her already entered.
Since not much data can be entered in this form,
I think it is fine.

* Show Errors for Fixture Type in AddFixtures

Also delete TODO for Enter on Numeric Fields to
advance to the next field - this is surpsrisingly hard because
JS cannot hook into the native tabIndex.

* Simplify onSubmit in AddFixtures

* Add Error Tooltip for DmxMode in AddFixtures

* Replace Ref for focus by react-hook-form

* Move from useState to watch in reacht-hook-form

* A Little Testing on Form Validation

* Refactor NavbarExitWithTitle into Single Component

* Refactor into ValidatedSuggest

* Refactor NewFixture into Multiple Files

* Install styled-components

For example, it is used in two places, showing off how to
use it inline with the css prop and with the styled interface.

When using the css prop, an import must be manually added:
`import { } from 'styled-components/macro';`

* Refactors to Improve Readability

* Refactor Blueprint Variables API

* Clean Up A Bit in Cypress Folder

* Update CONTRIBUTING with Frontend Info

* Reduce Windows-Linux Redundancy in CONTRIBUTING

* Update to Cypress 8.3.1

* Fix Failing Tests in Chromium

This is a strange issue - clicking on the label and typing does work in
Firefox but not in Chromium. But the issue only shows up when
something is already typed into the field - it works for empty inputs.

Now we just directly type into the input associated with the label,
which works. The code for the utility function to do this is taken from
https://glebbahmutov.com/cypress-examples/6.5.0/recipes/form-input-by-label.html#reusable-function

* Remove Force-Click in Cypress Test

Don't know why it works without force now but I'm happy it does.

* Add Confirmation Dialog when Removing GDTF

* Fix Bug when Re-Adding GDTF

Steps to reproduce fixed bug:
(do not reload between steps)
- Add GDTF file
- Remove GDTF
- Try to add it again

Oberseved before fix: Nothing happens when trying to add again - have to
reload to make it work.

Expected and observed after fix: GDTF is added again the second time

* Move TODO to Issue #57

* Fix Remove GDTF Hotkey Label
@Firionus Firionus added this to the GlowDTF milestone Dec 1, 2021
@Firionus Firionus removed this from the GlowDTF milestone Dec 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant