feat: JS API reconnect #1149
feat: JS API reconnect #1149
Conversation
- Doesn't work correctly yet - Not sure how best to test locally
- Added unit tests - Display a message in Console when disconnected
Codecov Report
@@ Coverage Diff @@
## main #1149 +/- ##
==========================================
+ Coverage 44.15% 44.19% +0.03%
==========================================
Files 447 448 +1
Lines 33267 33310 +43
Branches 8356 8364 +8
==========================================
+ Hits 14688 14720 +32
- Misses 18530 18541 +11
Partials 49 49
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
I left the window in the disconnected state for 2 minutes and when re-enabling network, the "Attempting to reconnect" stays visible. At some point, it attempts to reconnect, but the credentials are invalid. That info should be propagated to the user and prompt them to reload the page to reauthenticate
That error was an uncaught promise though, so might need to be propagated through the JS API in a cleaner way.
Uncaught (in promise) Error: Authentication details invalid
onEnd http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-internal.js:1
unary http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-internal.js:1
rawOnEnd http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-internal.js:1
rawOnEnd http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-internal.js:1
onTransportEnd http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-internal.js:1
$onMessage http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-core.js:19095
handleEvent_2 http://8f2c-50-24-89-157.ngrok.io/jsapi/dh-core.js:19237
| addEventListener: jest.fn(), | ||
| subscribeToFieldUpdates: jest.fn(() => () => null), | ||
| removeEventListener: jest.fn(), | ||
| getTable: jest.fn(), | ||
| getObject: jest.fn(), | ||
| runCode: jest.fn(), | ||
| }; | ||
| } as unknown) as IdeSession; |
There was a problem hiding this comment.
Would be better to just mock all of IdeSession if it's not a pain. Shouldn't really affect the tests, but if IdeSession changes shape we don't know about it in the tests unless they fail
There was a problem hiding this comment.
It's a bit of a pain in the ass. class Table doesn't match the types, missing many methods, and doesn't see the static events (since our typings expect them to be properties instead of static). Cleaning up mock should happen after we start using the actual types published by JS API.
I did a bit of cleanup, but was going down a bit of a yak shaving exercise.
- Mock IdeSession a bit more, add some more mock methods for Table - Will finish cleanup after proper types are published - Listen to RECONNECT_AUTH_FAILED event
- TableCsvExporter was depending on Table not having a `freeze` method
- Informs the user an error has occurred and they can press Refresh to try again
EVENT_DISCONNECTandEVENT_RECONNECTon the connection, and display a "Reconnecting..." message in the console.HACK_CONNECTION_FAILURE(it's been deprecated).SHUTDOWNevent and display a message after shutdown.Testing steps:
ngrok http 10000VITE_CORE_API_URL=http://acfc-23-233-0-34.ngrok.io/jsapi npm start