Conversation
🦋 Changeset is good to goLatest commit: f11c264 We got this. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The preview packages of this pull request have been published. @bigtest/agentInstall using the following command: $ npm install @bigtest/agent@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/agent": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/atomInstall using the following command: $ npm install @bigtest/atom@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/atom": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/bundlerInstall using the following command: $ npm install @bigtest/bundler@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/bundler": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/cliInstall using the following command: $ npm install @bigtest/cli@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/cli": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/clientInstall using the following command: $ npm install @bigtest/client@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/client": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/effectionInstall using the following command: $ npm install @bigtest/effection@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/effection": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/effection-expressInstall using the following command: $ npm install @bigtest/effection-express@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/effection-express": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/serverInstall using the following command: $ npm install @bigtest/server@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/server": "tm_better-error-when-bigtest-server-is-not-available"
}@bigtest/webdriverInstall using the following command: $ npm install @bigtest/webdriver@tm_better-error-when-bigtest-server-is-not-availableOr update your package.json file: {
"@bigtest/webdriver": "tm_better-error-when-bigtest-server-is-not-available"
} |
cowboyd
left a comment
cowboyd
left a comment
There was a problem hiding this comment.
I'm wondering if we should have a timeout
| if (e.message.includes('websocket server closed connection unexpectedly')) { | ||
| throw new MainError({ | ||
| exitCode: 1, | ||
| message: `Could not connect to BigTest server on ${uri}. Run "bigtest server" to start the server.` |
There was a problem hiding this comment.
Since we use the runTest command in both the ci and test commands, we have a potential weird case where the server stops responding to commands in between when it is started, and when we actually try to connect the client. This should be exceedingly rare, but I figured I'd point it out nonetheless.
Co-authored-by: Charles Lowell <cowboyd@frontside.com>
taras
force-pushed
the
tm/better-error-when-bigtest-server-is-not-available
branch
from
06f9257 to
baec97c
Compare
| yield spawn(function* detectStartupError(): Operation<void> { | ||
| let [error] = yield once(socket, 'error'); | ||
|
|
||
| if (isYaetiError(error)) { |
There was a problem hiding this comment.
I'd call this isErrorEvent since Yaeti is just the nodejs error event emulator for the ErrorEvent. When the client is running on the browser, it will actually be an ErrorEvent
There was a problem hiding this comment.
Worth noting that on the web, this will always be an error event.
cowboyd
left a comment
cowboyd
left a comment
There was a problem hiding this comment.
Looks great! effection code looking sharp.
| if (isErrorEvent(error)) { | ||
| throw new NoServerError(`Could not connect to server at ${url}`); | ||
| } else { | ||
| throw error; |
There was a problem hiding this comment.
I'm trying to imagine a scenario where we execute this code branch, and can't think of one other than a bug in the websocket library. Still, it's good to have in case there is and it will throw a big nasty stack trace.
| @@ -16,13 +17,25 @@ export class Client { | |||
| static *create(url: string): Operation<Client> { | |||
| let socket = new w3cwebsocket(url) as WebSocket; | |||
There was a problem hiding this comment.
Out of scope for this PR, but in order to make this work in the browser, we're going to have to loosely couple to our underlying websocket library. I wonder what is the best way to do that.
4 participants
Closes #427
Motivation
People should know what is happening when an error occurred. They should not have to look at stack traces to figure out what it means. When running tests without bigtest server present, we would get an ugly stack trace that said:
websocket server closed connection unexpectedly. Instead, we want to show an error that suggests to runbigtest serverbefore running tests.Approach
Detect when we get
websocket server closed connection unexpectedlyand show error:Could not connect to BigTest server on ${uri}. Run "bigtest server" to start the server.instead.