Skip to content

Commit

Permalink
🧹 Make retry a stable feature (LayerZero-Labs#622)
Browse files Browse the repository at this point in the history
  • Loading branch information
@janjakubnanista
janjakubnanista authored May 31, 2024
1 parent 7db7fc5 commit 87e1704
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 170 deletions.
6 changes: 6 additions & 0 deletions .changeset/pretty-suits-enjoy.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@layerzerolabs/devtools": patch
"@layerzerolabs/toolbox-hardhat": patch
---

Remove LZ_ENABLE_EXPERIMENTAL_RETRY feature flag
14 changes: 0 additions & 14 deletions EXPERIMENTAL.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ By default, the RPC calls that check the current state of your contracts are exe

Parallel execution can improve the speed of this process significantly. However, since it requires a large number of RPC calls to be executed simultaneously, it can result in `HTTP 429 Too Many Requests` RPC errors when used with public RPCs.

In general, we recommend combining this feature with <a href="#automatic-retries">automatic retries</a>.

### To enable

`LZ_ENABLE_EXPERIMENTAL_PARALLEL_EXECUTION=1`
Expand All @@ -34,18 +32,6 @@ In general, we recommend combining this feature with <a href="#automatic-retries

`LZ_ENABLE_EXPERIMENTAL_PARALLEL_EXECUTION=`

## Automatic retries <a id="automatic-retries"></a>

By default, the RPC calls that check the current state of your contracts are executed without retrying any failed requests. This feature flag enables an exponential backoff retry functionality on the RPC reads.

### To enable

`LZ_ENABLE_EXPERIMENTAL_RETRY=1`

### To disable

`LZ_ENABLE_EXPERIMENTAL_RETRY=`

## Batched transaction sending <a id="batched-send"></a>

Some signers might support batched transaction sending (e.g. Gnosis Safe signer). If turned on, this feature flag will make use of the batched sending. If this feature flag is on and batched sending is not available, regular sending will be used instead.
Expand Down
3 changes: 1 addition & 2 deletions packages/devtools/src/common/retry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ export const createDefaultRetryHandler = (loggerName: string = 'AsyncRetriable')
}

export const AsyncRetriable = ({
// We'll feature flag this functionality for the time being
enabled = !!process.env.LZ_ENABLE_EXPERIMENTAL_RETRY,
enabled = true,
maxDelay,
numAttempts = 3,
onRetry = createDefaultRetryHandler(),
Expand Down
231 changes: 77 additions & 154 deletions packages/devtools/test/common/retry.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,185 +2,108 @@ import { AsyncRetriable } from '@/common/retry'

describe('common/retry', () => {
describe('AsyncRetriable', () => {
describe('when LZ_ENABLE_EXPERIMENTAL_RETRY is off', () => {
beforeAll(() => {
// We'll enable the AsyncRetriable for these tests
process.env.LZ_ENABLE_EXPERIMENTAL_RETRY = ''
})

it('shoult not retry', async () => {
const error = new Error('Told ya')
const handleRetry = jest.fn()
const mock = jest.fn().mockRejectedValue(error)

class WithAsyncRetriable {
@AsyncRetriable({ onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
it('should retry a method call 3 times by default', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)

class WithAsyncRetriable {
@AsyncRetriable()
async iAlwaysFail(value: string) {
return mock(value)
}
}

await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)
await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

expect(mock).toHaveBeenCalledTimes(1)
expect(handleRetry).not.toHaveBeenCalled()
})

it('should retry if enabled is set to true', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)

class WithAsyncRetriable {
@AsyncRetriable({ enabled: true })
async iAlwaysFail(value: string) {
return mock(value)
}
}

await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

expect(mock).toHaveBeenCalledTimes(3)
expect(mock).toHaveBeenNthCalledWith(1, 'y')
expect(mock).toHaveBeenNthCalledWith(2, 'y')
expect(mock).toHaveBeenNthCalledWith(3, 'y')
})

it('should have access to non-prototype properties', async () => {
const mock = jest.fn().mockResolvedValue(true)
expect(mock).toHaveBeenCalledTimes(3)
expect(mock).toHaveBeenNthCalledWith(1, 'y')
expect(mock).toHaveBeenNthCalledWith(2, 'y')
expect(mock).toHaveBeenNthCalledWith(3, 'y')
})

class WithAsyncRetriable {
constructor(protected readonly property = 7) {}
it('should retry a method call N times if numAttempts is specified', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)

@AsyncRetriable({ enabled: true })
async iHaveAccessToProperties() {
return mock(this.property)
}
class WithAsyncRetriable {
@AsyncRetriable({ numAttempts: 2 })
async iAlwaysFail(value: string) {
return mock(value)
}
}

await expect(new WithAsyncRetriable().iHaveAccessToProperties()).resolves.toBe(true)
await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

expect(mock).toHaveBeenCalledTimes(1)
expect(mock).toHaveBeenNthCalledWith(1, 7)
})
expect(mock).toHaveBeenCalledTimes(2)
})

describe('when LZ_ENABLE_EXPERIMENTAL_RETRY is on', () => {
beforeAll(() => {
// We'll enable the AsyncRetriable for these tests
process.env.LZ_ENABLE_EXPERIMENTAL_RETRY = '1'
})

afterAll(() => {
process.env.LZ_ENABLE_EXPERIMENTAL_RETRY = ''
})

it('should retry a method call 3 times by default', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)

class WithAsyncRetriable {
@AsyncRetriable()
async iAlwaysFail(value: string) {
return mock(value)
}
it('should stop retrying if the onRetry handler returns false', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)
const handleRetry = jest
.fn()
// We check that if we return undefined/void we'll keep trying
.mockReturnValueOnce(undefined)
// We check that if we return true we keep trying
.mockReturnValueOnce(true)
// After the third attempt we return false
.mockReturnValueOnce(false)

class WithAsyncRetriable {
@AsyncRetriable({ numAttempts: 10_000, onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
}

await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

expect(mock).toHaveBeenCalledTimes(3)
expect(mock).toHaveBeenNthCalledWith(1, 'y')
expect(mock).toHaveBeenNthCalledWith(2, 'y')
expect(mock).toHaveBeenNthCalledWith(3, 'y')
})
await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

it('should retry a method call N times if numAttempts is specified', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)
expect(mock).toHaveBeenCalledTimes(3)
expect(handleRetry).toHaveBeenCalledTimes(3)
})

class WithAsyncRetriable {
@AsyncRetriable({ numAttempts: 2 })
async iAlwaysFail(value: string) {
return mock(value)
}
}
it('should call the onRetry callback if provided', async () => {
const error = new Error('Told ya')
const handleRetry = jest.fn()
const mock = jest.fn().mockRejectedValue(error)

await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)

expect(mock).toHaveBeenCalledTimes(2)
})

it('should stop retrying if the onRetry handler returns false', async () => {
const error = new Error('Told ya')
const mock = jest.fn().mockRejectedValue(error)
const handleRetry = jest
.fn()
// We check that if we return undefined/void we'll keep trying
.mockReturnValueOnce(undefined)
// We check that if we return true we keep trying
.mockReturnValueOnce(true)
// After the third attempt we return false
.mockReturnValueOnce(false)

class WithAsyncRetriable {
@AsyncRetriable({ numAttempts: 10_000, onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
class WithAsyncRetriable {
@AsyncRetriable({ onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
}

await expect(new WithAsyncRetriable().iAlwaysFail('y')).rejects.toBe(error)
const withAsyncRetriable = new WithAsyncRetriable()

expect(mock).toHaveBeenCalledTimes(3)
expect(handleRetry).toHaveBeenCalledTimes(3)
})
await expect(withAsyncRetriable.iAlwaysFail('y')).rejects.toBe(error)

it('should call the onRetry callback if provided', async () => {
const error = new Error('Told ya')
const handleRetry = jest.fn()
const mock = jest.fn().mockRejectedValue(error)
expect(handleRetry).toHaveBeenCalledTimes(3)
expect(handleRetry).toHaveBeenNthCalledWith(1, 1, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(2, 2, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(3, 3, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
})

class WithAsyncRetriable {
@AsyncRetriable({ onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
}
it('should resolve if the method resolves within the specified number of attempts', async () => {
const error = new Error('Told ya')
const value = {}
const handleRetry = jest.fn()
const mock = jest.fn().mockRejectedValueOnce(error).mockRejectedValueOnce(error).mockResolvedValue(value)

const withAsyncRetriable = new WithAsyncRetriable()

await expect(withAsyncRetriable.iAlwaysFail('y')).rejects.toBe(error)

expect(handleRetry).toHaveBeenCalledTimes(3)
expect(handleRetry).toHaveBeenNthCalledWith(1, 1, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(2, 2, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(3, 3, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
})

it('should resolve if the method resolves within the specified number of attempts', async () => {
const error = new Error('Told ya')
const value = {}
const handleRetry = jest.fn()
const mock = jest
.fn()
.mockRejectedValueOnce(error)
.mockRejectedValueOnce(error)
.mockResolvedValue(value)

class WithAsyncRetriable {
@AsyncRetriable({ onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
class WithAsyncRetriable {
@AsyncRetriable({ onRetry: handleRetry })
async iAlwaysFail(value: string) {
return mock(value)
}
}

const withAsyncRetriable = new WithAsyncRetriable()
const withAsyncRetriable = new WithAsyncRetriable()

await expect(withAsyncRetriable.iAlwaysFail('y')).resolves.toBe(value)
await expect(withAsyncRetriable.iAlwaysFail('y')).resolves.toBe(value)

expect(handleRetry).toHaveBeenCalledTimes(2)
expect(handleRetry).toHaveBeenNthCalledWith(1, 1, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(2, 2, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
})
expect(handleRetry).toHaveBeenCalledTimes(2)
expect(handleRetry).toHaveBeenNthCalledWith(1, 1, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
expect(handleRetry).toHaveBeenNthCalledWith(2, 2, 3, error, withAsyncRetriable, 'iAlwaysFail', ['y'])
})
})
})

0 comments on commit 87e1704

Please sign in to comment.