fix(proxy): refresh connection between retries #3566
Conversation
| if (!this.activityLogId) throw new Error('Parameter activityLogId is required'); | ||
| if (!this.environmentId) throw new Error('Parameter environmentId is required'); | ||
| if (!this.nangoConnectionId) throw new Error('Parameter nangoConnectionId is required'); | ||
| if (!this.syncConfig) throw new Error('Parameter syncConfig is required'); | ||
| } |
There was a problem hiding this comment.
left over from rewrite
| proxyConfig: config | ||
| }); | ||
|
|
||
| const responseStream = (await proxy.request({ axiosConfig })).unwrap(); |
There was a problem hiding this comment.
All the removed logic has been abstracted in getAxiosConfiguration
| /** | ||
| * Called on error, gives the ability to control the retry and wait time | ||
| */ | ||
| onError?: Props['onError']; |
| onError?: (args: { err: unknown; max: number; attempt: number }) => { retry: boolean; reason: string; wait?: number }; | ||
| getConnection: () => MaybePromise<ConnectionForProxy>; | ||
| } | ||
|
|
There was a problem hiding this comment.
Rewrote this file to accept a getConnection param that's called on every iteration
| @@ -6,36 +6,32 @@ import { Readable, Transform, PassThrough } from 'stream'; | |||
| import type { UrlWithParsedQuery } from 'url'; | |||
There was a problem hiding this comment.
Removed a lot of 'functional method' that did nothing
| const redactedURL = redactURL({ url: axiosConfig.url!, valuesToFilter }); | ||
| /** | ||
| * For testing purpose only | ||
| * @private |
There was a problem hiding this comment.
How does this @Private work? The method is still declared as public below
There was a problem hiding this comment.
it's only for documentation purposes, I need it to be public so I can mock it
| @@ -26,23 +33,51 @@ export class ProxyError extends Error { | |||
| } | |||
| } | |||
|
|
|||
| const methodDataAllowed = ['POST', 'PUT', 'PATCH', 'DELETE']; | |||
There was a problem hiding this comment.
used only once. Not sure it requires its own global const
There was a problem hiding this comment.
true, but allocating an array at each call seems like a waste to me
This reverts commit 85c998b.
Changes
Fixes https://linear.app/nango/issue/NAN-2745/retry-strategy-should-call-getconnection-before-each-try
ProxyRequest now accepts a
getConnectionparam called on every iterationThe main idea behind this PR is to re-inject the connection after TTL expires.
I had to rewrite part of the code to be able to dynamically build axios config (url + headers).
Type: Split ProxyConfig and Connection
Passing a connection to this type was not useful and was kind of weird with the new behavior. Now we pass a base proxyconfig that tells what we want to query and we recompile the axios config based on that.
Tests
Very long retry with a periodic refresh
