Skip to content

Commit 9677e21

Browse files
Cleanup closehandler and add timeout for testSFTPAuthentication
1 parent 16ec329 commit 9677e21

File tree

2 files changed

+24
-16
lines changed

2 files changed

+24
-16
lines changed

packages/destination-actions/src/destinations/sftp/client.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { PayloadValidationError, RequestTimeoutError, Logger, IntegrationError } from '@segment/actions-core'
1+
import {
2+
PayloadValidationError,
3+
RequestTimeoutError,
4+
Logger,
5+
IntegrationError,
6+
DEFAULT_REQUEST_TIMEOUT
7+
} from '@segment/actions-core'
28
import path from 'path'
39
import Client from 'ssh2-sftp-client'
410
import { SFTP_DEFAULT_PORT } from './constants'
@@ -160,7 +166,18 @@ function normalizeSSHKey(key = ''): string {
160166
*/
161167
async function testSFTPConnection(settings: Settings): Promise<unknown> {
162168
const sftp = new Client()
163-
let res
169+
const timeoutPromise = new Promise((_, reject) => {
170+
setTimeout(() => {
171+
reject(new RequestTimeoutError('SFTP connection timed out'))
172+
}, DEFAULT_REQUEST_TIMEOUT)
173+
})
174+
// Throws if connection or listing fails
175+
// Otherwise resolves with the list of files in the root directory
176+
return Promise.race([timeoutPromise, connectAndList(sftp, settings)])
177+
}
178+
179+
async function connectAndList(sftp: Client, settings: Settings): Promise<Client.FileInfo[]> {
180+
let res: Client.FileInfo[]
164181
try {
165182
await sftp.connect(createConnectionConfig(settings))
166183
res = await sftp.list('/')

packages/destination-actions/src/destinations/sftp/sftp-wrapper.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,26 +105,17 @@ export class SFTPWrapper {
105105
await Promise.all(writeRequests)
106106
}
107107

108-
const closeHandle = (): Promise<void> => {
109-
return new Promise((closeResolve) => {
108+
processWrites()
109+
.then(resolve)
110+
.catch(resolve)
111+
.finally(() => {
112+
// we don't care if close fails here because we expect the caller to call sftp.end() eventually
110113
this.client?.close(handle, (closeErr) => {
111114
if (closeErr) {
112115
this.logger?.warn('Error closing remote file handle:', String(closeErr.message))
113116
}
114-
closeResolve()
115117
})
116118
})
117-
}
118-
119-
processWrites()
120-
.then(async () => {
121-
await closeHandle()
122-
resolve()
123-
})
124-
.catch(async (err) => {
125-
await closeHandle()
126-
reject(err)
127-
})
128119
})
129120
})
130121
}

0 commit comments

Comments
 (0)