This repository was archived by the owner on Dec 18, 2018. It is now read-only.
Implement #1162 by adding client timeout for JavaScript #1163
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
analogrelay
changed the title
Contributor
Author
|
🆙 📅 Ready for review |
| send(data: any): Promise<void> { | ||
| return Promise.reject(""); | ||
| }, | ||
| type: TransportType.LongPolling, |
Contributor
Author
There was a problem hiding this comment.
Yep, old code, not cleaned up :). Thanks!
| connect(url: string, requestedTransferMode: TransferMode): Promise<TransferMode> { return Promise.resolve(transportTransferMode); }, | ||
| send(data: any): Promise<void> { return Promise.resolve(); }, | ||
| stop(): void {}, | ||
| type: TransportType.LongPolling, |
| type: TransportType.LongPolling, | ||
| onreceive: null, | ||
| onclose: null, | ||
| mode: transportTransferMode |
| class TestConnection implements IConnection { | ||
| readonly features: any = {}; | ||
|
|
||
| transportType: TransportType = TransportType.LongPolling; |
| import { asyncit as it, captureException } from './JasmineUtils'; | ||
| import { asyncit as it, captureException, delay, PromiseSource } from './Utils'; | ||
| import { IHubConnectionOptions } from "../Microsoft.AspNetCore.SignalR.Client.TS/IHubConnectionOptions"; | ||
| import { connect } from "tls"; |
Contributor
Author
There was a problem hiding this comment.
VSCode being overly "helpful" and trying to import things I accidentally choose in Intellisense
| }); | ||
| }); | ||
|
|
||
| if(transportType != signalR.TransportType.LongPolling) { |
| import { HubConnection } from "../Microsoft.AspNetCore.SignalR.Client.TS/HubConnection" | ||
| import { DataReceived, ConnectionClosed } from "../Microsoft.AspNetCore.SignalR.Client.TS/Common" | ||
| import { TransportType, ITransport, TransferMode } from "../Microsoft.AspNetCore.SignalR.Client.TS/Transports" | ||
| import { TransportType, ITransport, TransferMode, LongPollingTransport } from "../Microsoft.AspNetCore.SignalR.Client.TS/Transports" |
| fail(e); | ||
| done(); | ||
| }); | ||
| .then(function () { |
Member
There was a problem hiding this comment.
I liked the old indentation
Contributor
Author
There was a problem hiding this comment.
That's the VSCode auto-formatter at work :(.
| let guidBytes = []; | ||
| for (let i = 0; i < value.GUID.length; i++) { | ||
| guidBytes.push(value.GUID.charCodeAt(i)); | ||
| .then(function (value) { |
Contributor
Author
|
🆙 📅 clean-up |
BrennanConroy
approved these changes
Nov 28, 2017
| } | ||
|
|
||
| private processIncomingData(data: any) { | ||
| // For some reason, our TypeScript refs are set up for clearTimeout to take NodeJS.Timer... which is weird. |
Member
There was a problem hiding this comment.
Is this a concern for us?
Contributor
Author
There was a problem hiding this comment.
Actually, I figured this out and it's intentional. This time around we're building our JS client as a NodeJS library and using Browserify to polyfill, rather than as a browser library, so we do want to use the NodeJS timer pattern.
Contributor
Author
|
FYI, not merging this until #1161 is merged, as mentioned above. |
analogrelay
force-pushed
the
anurse/1162-js-client-timeout
branch
from
0dbda11 to
6d2b466
Compare
analogrelay
force-pushed
the
anurse/1162-js-client-timeout
branch
from
6d2b466 to
fa0ccb2
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add client timeout to JS client. It is configurable via an option and defaults to 30 seconds (the server ping rate defaults to 15 seconds, so this is effectively 2 server ping intervals).
NOTE: Should not be merged until #1161 is merged since it will immediately break any connection that goes idle for 30 seconds :).
Fixes #1162