-
Notifications
You must be signed in to change notification settings - Fork 140
Snow 1944643 network errors retry #1015
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
base: master
Are you sure you want to change the base?
Changes from all commits
705292c
49a0792
fe82a6a
509f401
4cd3bff
9f81cb4
2be1e28
dd2175b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* Copyright (c) 2015-2024 Snowflake Computing Inc. All rights reserved. | ||
*/ | ||
|
||
const { runWireMockAsync, addWireMockMappingsFromFile } = require('../wiremockRunner'); | ||
const connParameters = require('../authentication/connectionParameters'); | ||
const testUtil = require('../integration/testUtil'); | ||
const snowflake = require('../../lib/snowflake'); | ||
const assert = require('assert'); | ||
|
||
describe('Connection test', function () { | ||
this.timeout(500000); | ||
let port; | ||
let wireMock; | ||
|
||
before(async () => { | ||
port = await testUtil.getFreePort(); | ||
wireMock = await runWireMockAsync(port); | ||
snowflake.configure({ | ||
logLevel: 'DEBUG', | ||
disableOCSPChecks: true | ||
}); | ||
}); | ||
|
||
afterEach(async () => { | ||
wireMock.scenarios.resetAllScenarios(); | ||
}); | ||
|
||
after(async () => { | ||
await wireMock.global.shutdown(); | ||
}); | ||
|
||
it('Test retries after connection reset - success', async function () { | ||
await addWireMockMappingsFromFile(wireMock, 'wiremock/mappings/six_reset_connection_and_correct_response.json'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based only on mapping file name - should we also have a test, when it eventually ends up in connection reset propagated to the user? |
||
const connectionOption = { ...connParameters.wiremock, password: 'MOCK_TOKEN', port: port, sfRetryMaxSleepTime: 2, sfRetryMaxNumRetries: 10 }; | ||
const connection = testUtil.createConnection(connectionOption); | ||
await testUtil.connectAsync(connection); | ||
await assert.doesNotReject(async () => await testUtil.executeCmdAsync(connection, ' Select 1')); | ||
}); | ||
|
||
it('Test retries after malformed response', async function () { | ||
await addWireMockMappingsFromFile(wireMock, 'wiremock/mappings/six_malformed_and_correct.json'); | ||
const connectionOption = { ...connParameters.wiremock, password: 'MOCK_TOKEN', port: port, sfRetryMaxSleepTime: 2 }; | ||
const connection = testUtil.createConnection(connectionOption); | ||
await testUtil.connectAsync(connection); | ||
await assert.doesNotReject(async () => await testUtil.executeCmdAsync(connection, ' Select 1')); | ||
}); | ||
|
||
it('Test retries after connection reset - fail', async function () { | ||
await addWireMockMappingsFromFile(wireMock, 'wiremock/mappings/six_reset_connection_and_correct_response.json'); | ||
const connectionOption = { ...connParameters.wiremock, password: 'MOCK_TOKEN', port: port, sfRetryMaxNumRetries: 1, sfRetryMaxSleepTime: 2 }; | ||
const connection = testUtil.createConnection(connectionOption); | ||
await testUtil.connectAsync(connection); | ||
await assert.rejects( | ||
testUtil.executeCmdAsync(connection, ' Select 1'), | ||
(err) => { | ||
assert.match(err.message, /Network error. Could not reach Snowflake./); | ||
return true; | ||
}, | ||
); | ||
}); | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,15 @@ | ||
const assert = require('assert'); | ||
const fs = require('fs'); | ||
const net = require('net'); | ||
const axios = require('axios'); | ||
const { runWireMockAsync } = require('../../wiremockRunner'); | ||
const os = require('os'); | ||
|
||
async function getFreePort() { | ||
return new Promise(res => { | ||
const srv = net.createServer(); | ||
srv.listen(0, () => { | ||
const port = srv.address().port; | ||
srv.close(() => res(port)); | ||
}); | ||
}); | ||
} | ||
const testUtil = require('../testUtil'); | ||
|
||
if (os.platform !== 'win32') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it a test for wiremock itself? Do we still need this? |
||
describe('Wiremock test', function () { | ||
let port, wireMock; | ||
before(async () => { | ||
port = await getFreePort(); | ||
port = await testUtil.getFreePort(); | ||
wireMock = await runWireMockAsync(port); | ||
}); | ||
after(async () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since now
PARAM_RETRY_SF_MAX_NUM_RETRIES
,PARAM_RETRY_SF_STARTING_SLEEP_TIME
, andPARAM_RETRY_SF_MAX_SLEEP_TIME
will be public, my respectful request here would be that please make sure the parameters are correctly documented in Snowflake docs. Thank you in advance! 🙇