Skip to content

Commit

Permalink
Fix connection check now that we're using a proxy (#388)
Browse files Browse the repository at this point in the history
After #386,
this connection check no longer makes sense as we can always open a port
to the proxy so it isn't really checking anything. Instead, let's try
and actually fetch the appUrl and see if the server behind the proxy
gives us _something_ back. We don't check the status code other than 502
which is what the proxy will return if it can't connect to the server
since this most closely mimics the behaviour of just opening a socket
that we had before.

This PR also adds in some waiting, so that we'll retry a couple of times
to allow for the fact that the server to test against may take a bit of
time to spin up. This should avoid the need for customers to add/tweak
manual `sleep`s in their workflow.

Also, one small fix to an error message.
  • Loading branch information
edoardopirovano committed Feb 7, 2024
2 parents 1f5837e + 83bfed2 commit ae55f4a
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 28 deletions.
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"@sentry/node": "^7.50.0",
"lodash.debounce": "^4.0.8",
"loglevel": "^1.8.1",
"luxon": "^3.4.3"
"luxon": "^3.4.3",
"retry": "^0.13.1"
},
"devDependencies": {
"@alwaysmeticulous/api": "^2.97.0",
Expand All @@ -43,6 +44,7 @@
"@types/jest": "^27.0.3",
"@types/lodash.debounce": "^4.0.7",
"@types/luxon": "^3.3.2",
"@types/retry": "^0.12.5",
"@types/node": "^20.2.5",
"@typescript-eslint/eslint-plugin": "^5.62.0",
"@typescript-eslint/parser": "^5.62.0",
Expand Down
63 changes: 37 additions & 26 deletions src/utils/check-connection.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,44 @@
import { Socket } from "net";
import * as retry from "retry";

export const throwIfCannotConnectToOrigin = async (url: string) => {
const { hostname, port, protocol, origin } = new URL(url);
const defaultPortForProtocol = protocol === "https:" ? 443 : 80;
const portNumber =
port != null && port != "" ? Number(port) : defaultPortForProtocol;
const connectionAccepted = await canConnectTo(hostname, portNumber);
if (!connectionAccepted) {
throw new Error(
`Could not connect to '${hostname}:${portNumber}'. Please check:\n\n` +
`1. The server running at '${origin}' has fully started by the time the Meticulous action starts. You may need to add a 'sleep 30' after starting the server to ensure that this is the case.\n` +
`2. The server running at '${origin}' is using tcp instead of tcp6. You can use 'netstat -tulpen' to see what addresses and ports it is bound to.\n\n`
export const throwIfCannotConnectToOrigin = async (appUrl: string) => {
// Wait 1s, 2s, 4s, 8s, 16s, 32s, 64s for a total of just over 2 minutes
const operation = retry.operation({
retries: 7,
factor: 2,
minTimeout: 1_000,
});
const url = new URL(appUrl);
operation.attempt(async () => {
if (await canConnectTo(url)) {
return;
}
operation.retry(
new Error(
`Could not connect to '${appUrl}'. Please check:\n\n` +
`1. The server running at '${origin}' has fully started by the time the Meticulous action starts. You may need to add a 'sleep 30' after starting the server to ensure that this is the case.\n` +
`2. The server running at '${origin}' is using tcp instead of tcp6. You can use 'netstat -tulpen' to see what addresses and ports it is bound to.\n\n`
)
);
});
};

const canConnectTo = async (url: URL) => {
try {
const result = await fetchWithTimeout(url);
return result.status !== 502;
} catch (error) {
return false;
}
};

const canConnectTo = async (host: string, port: number, timeout = 5000) => {
return new Promise((resolve) => {
const socket = new Socket();
const onError = () => {
socket.destroy();
resolve(false);
};
async function fetchWithTimeout(url: URL) {
const controller = new AbortController();
const id = setTimeout(() => controller.abort(), 5000);

socket.setTimeout(timeout, onError);
socket.on("error", onError);
socket.connect(port, host, () => {
socket.end();
resolve(true);
});
const response = await fetch(url, {
signal: controller.signal,
});
};
clearTimeout(id);

return response;
}
2 changes: 1 addition & 1 deletion src/utils/results-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class ResultsReporter {
// Usually this means that the user has just set up Meticulous and is running it for the first time.
await this.setStatusComment({
createIfDoesNotExist: true,
body: `🤖 Meticulous replayed ${testCaseResults.length} user sessions and [took ${totalScreenshotsTaken} visual snapshots](${testRun.url}). Meticulous did not run on ${this.options.baseRef} of the ${baseRefStr} branch and so there was nothing to compare against.
body: `🤖 Meticulous replayed ${testCaseResults.length} user sessions and [took ${totalScreenshotsTaken} visual snapshots](${testRun.url}). Meticulous did not run on ${this.options.baseSha} of the ${baseRefStr} branch and so there was nothing to compare against.
\nPlease merge your pull request for setting up Meticulous in CI and ensure that it’s running on push events to the ${baseRefStr} branch.`,
});
}
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1772,6 +1772,11 @@
resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.7.2.tgz#6c2324641cc4ba050a8c710b2b251b377581fbf0"
integrity sha512-KufADq8uQqo1pYKVIYzfKbJfBAc0sOeXqGbFaSpv8MRmC/zXgowNZmFcbngndGk922QDmOASEXUZCaY48gs4cg==

"@types/retry@^0.12.5":
version "0.12.5"
resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.5.tgz#f090ff4bd8d2e5b940ff270ab39fd5ca1834a07e"
integrity sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw==

"@types/semver@^7.3.12":
version "7.3.13"
resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.3.13.tgz#da4bfd73f49bd541d28920ab0e2bf0ee80f71c91"
Expand Down Expand Up @@ -5297,6 +5302,11 @@ retry@^0.12.0:
resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b"
integrity sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==

retry@^0.13.1:
version "0.13.1"
resolved "https://registry.yarnpkg.com/retry/-/retry-0.13.1.tgz#185b1587acf67919d63b357349e03537b2484658"
integrity sha512-XQBQ3I8W1Cge0Seh+6gjj03LbmRFWuoszgK9ooCpwYIrhhoO80pfq4cUkU5DkknwfOfFteRwlZ56PYOGYyFWdg==

reusify@^1.0.4:
version "1.0.4"
resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.0.4.tgz#90da382b1e126efc02146e90845a88db12925d76"
Expand Down

0 comments on commit ae55f4a

Please sign in to comment.