Skip to content

Commit e72e19f

Browse files
MaxAakeMaxAake
authored
Deprecate isRetriableError and ensure isRetryableError functions (#1305)
1 parent 9175980 commit e72e19f

File tree

22 files changed

+161
-48
lines changed

22 files changed

+161
-48
lines changed

‎packages/bolt-connection/src/connection-provider/connection-provider-pooled.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ export default class PooledConnectionProvider extends ConnectionProvider {
249249
const handled = this._authenticationProvider.handleError({ connection, code: error.code })
250250

251251
if (handled) {
252-
error.retriable = true
252+
error.retriable = true // remove in 7.0
253+
error.retryable = true
253254
}
254255

255256
if (error.code === 'Neo.ClientError.Security.AuthorizationExpired') {

‎packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ it('should call authenticationAuthProvider.handleError when TokenExpired happens
205205
expect(handleError).toBeCalledWith({ connection: conn, code: 'Neo.ClientError.Security.TokenExpired' })
206206
})
207207

208-
it('should change error to retriable when error when TokenExpired happens and staticAuthTokenManager is not being used', async () => {
208+
it('should change error to retryable when error when TokenExpired happens and staticAuthTokenManager is not being used', async () => {
209209
const address = ServerAddress.fromUrl('localhost:123')
210210
const pool = newPool()
211211
const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.bearer({ tokenProvider: () => null }))
@@ -223,9 +223,10 @@ it('should change error to retriable when error when TokenExpired happens and st
223223
const error = conn.handleAndTransformError(expectedError, address)
224224

225225
expect(error.retriable).toBe(true)
226+
expect(error.retryable).toBe(true)
226227
})
227228

228-
it('should not change error to retriable when error when TokenExpired happens and staticAuthTokenManager is being used', async () => {
229+
it('should not change error to retryable when error when TokenExpired happens and staticAuthTokenManager is being used', async () => {
229230
const address = ServerAddress.fromUrl('localhost:123')
230231
const pool = newPool()
231232
const connectionProvider = newDirectConnectionProvider(address, pool, staticAuthTokenManager({ authToken: null }))
@@ -243,9 +244,10 @@ it('should not change error to retriable when error when TokenExpired happens an
243244
const error = conn.handleAndTransformError(expectedError, address)
244245

245246
expect(error.retriable).toBe(false)
247+
expect(error.retryable).toBe(false)
246248
})
247249

248-
it('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used', async () => {
250+
it('should not change error to retryable when error when TokenExpired happens and authTokenManagers.basic is being used', async () => {
249251
const address = ServerAddress.fromUrl('localhost:123')
250252
const pool = newPool()
251253
const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.basic({ tokenProvider: () => null }))
@@ -263,6 +265,7 @@ it('should not change error to retriable when error when TokenExpired happens an
263265
const error = conn.handleAndTransformError(expectedError, address)
264266

265267
expect(error.retriable).toBe(false)
268+
expect(error.retryable).toBe(false)
266269
})
267270

268271
describe('constructor', () => {

‎packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,7 +1704,7 @@ describe.each([
17041704
expect(error).toBe(expectedError)
17051705
})
17061706

1707-
it.each(usersDataSet)('should change error to retriable when error when TokenExpired happens and staticAuthTokenManager is not being used [user=%s]', async (user) => {
1707+
it.each(usersDataSet)('should change error to retryable when error when TokenExpired happens and staticAuthTokenManager is not being used [user=%s]', async (user) => {
17081708
const pool = newPool()
17091709
const connectionProvider = newRoutingConnectionProvider(
17101710
[
@@ -1741,10 +1741,12 @@ describe.each([
17411741
const error2 = server2Connection.handleAndTransformError(error, server2)
17421742

17431743
expect(error1.retriable).toBe(true)
1744+
expect(error1.retryable).toBe(true)
17441745
expect(error2.retriable).toBe(true)
1746+
expect(error2.retryable).toBe(true)
17451747
})
17461748

1747-
it.each(usersDataSet)('should not change error to retriable when error when TokenExpired happens and staticAuthTokenManager is being used [user=%s]', async (user) => {
1749+
it.each(usersDataSet)('should not change error to retryable when error when TokenExpired happens and staticAuthTokenManager is being used [user=%s]', async (user) => {
17481750
const pool = newPool()
17491751
const connectionProvider = newRoutingConnectionProvider(
17501752
[
@@ -1782,10 +1784,12 @@ describe.each([
17821784
const error2 = server2Connection.handleAndTransformError(error, server2)
17831785

17841786
expect(error1.retriable).toBe(false)
1787+
expect(error1.retryable).toBe(false)
17851788
expect(error2.retriable).toBe(false)
1789+
expect(error2.retryable).toBe(false)
17861790
})
17871791

1788-
it.each(usersDataSet)('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used [user=%s]', async (user) => {
1792+
it.each(usersDataSet)('should not change error to retryable when error when TokenExpired happens and authTokenManagers.basic is being used [user=%s]', async (user) => {
17891793
const pool = newPool()
17901794
const connectionProvider = newRoutingConnectionProvider(
17911795
[
@@ -1823,7 +1827,9 @@ describe.each([
18231827
const error2 = server2Connection.handleAndTransformError(error, server2)
18241828

18251829
expect(error1.retriable).toBe(false)
1830+
expect(error1.retryable).toBe(false)
18261831
expect(error2.retriable).toBe(false)
1832+
expect(error2.retryable).toBe(false)
18271833
})
18281834

18291835
it.each(usersDataSet)('should use resolved seed router after accepting table with no writers [user=%s]', (user, done) => {

‎packages/bolt-connection/test/connection/connection-channel.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ describe('ChannelConnection', () => {
371371
expect(loggerFunction).toHaveBeenCalledWith(
372372
'error',
373373
`${connection} experienced a fatal error caused by Neo4jError: some error ` +
374-
'({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"C","retriable":false})'
374+
'({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"C","retriable":false,"retryable":false})'
375375
)
376376
})
377377
})
@@ -419,7 +419,7 @@ describe('ChannelConnection', () => {
419419
expect(loggerFunction).toHaveBeenCalledWith(
420420
'error',
421421
`${connection} experienced a fatal error caused by Neo4jError: current failure ` +
422-
'({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. current failure","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"ongoing","retriable":false})'
422+
'({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. current failure","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"ongoing","retriable":false,"retryable":false})'
423423
)
424424
})
425425
})

‎packages/core/src/driver.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ class Driver {
536536
}
537537

538538
/**
539-
* Executes a query in a retriable context and returns a {@link EagerResult}.
539+
* Executes a query in a retryable context and returns a {@link EagerResult}.
540540
*
541541
* This method is a shortcut for a {@link Session#executeRead} and {@link Session#executeWrite}.
542542
*

‎packages/core/src/error.ts

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,17 @@ class Neo4jError extends GQLError {
174174
/**
175175
* Whether the request that caused this error can be safely retried without duplicate commits on the server.
176176
* This does not apply when running auto-commit transactions using {@link Session#run}
177+
*
178+
* @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link retryable} instead.
177179
*/
178180
retriable: boolean
179181

182+
/**
183+
* Whether the request that caused this error can be safely retried without duplicate commits on the server.
184+
* This does not apply when running auto-commit transactions using {@link Session#run}
185+
*/
186+
retryable: boolean
187+
180188
/**
181189
* @constructor
182190
* @param {string} message - the error message
@@ -207,27 +215,49 @@ class Neo4jError extends GQLError {
207215
*/
208216
this.name = 'Neo4jError'
209217

218+
const isRetryableCode = _isRetryableCode(code)
210219
/**
211220
* If the error is considered retriable.
212221
* This does not apply when running auto-commit transactions using {@link Session#run}
213222
*
223+
* @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link retryable} instead.
224+
* @type {boolean}
225+
* @public
226+
*/
227+
this.retriable = isRetryableCode
228+
229+
/**
230+
* If the error is considered retryable.
231+
* This does not apply when running auto-commit transactions using {@link Session#run}
232+
*
214233
* @type {boolean}
215234
* @public
216235
*/
217-
this.retriable = _isRetriableCode(code)
236+
this.retryable = isRetryableCode
218237
}
219238

220239
/**
221240
* Verifies if the given error is retriable.
222241
*
242+
* @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link isRetryable} instead.
223243
* @param {object|undefined|null} error the error object
224244
* @returns {boolean} true if the error is retriable
225245
*/
226246
static isRetriable (error?: any | null): boolean {
247+
return this.isRetryable(error)
248+
}
249+
250+
/**
251+
* Verifies if the given error is retryable.
252+
*
253+
* @param {object|undefined|null} error the error object
254+
* @returns {boolean} true if the error is retryable
255+
*/
256+
static isRetryable (error?: any | null): boolean {
227257
return error !== null &&
228258
error !== undefined &&
229259
error instanceof Neo4jError &&
230-
error.retriable
260+
error.retryable
231261
}
232262
}
233263

@@ -263,18 +293,28 @@ function newGQLError (message: string, cause?: Error, gqlStatus?: string, gqlSta
263293
/**
264294
* Verifies if the given error is retriable.
265295
*
296+
* @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link isRetryableError} instead.
266297
* @public
267298
* @param {object|undefined|null} error the error object
268299
* @returns {boolean} true if the error is retriable
269300
*/
270301
const isRetriableError = Neo4jError.isRetriable
271302

303+
/**
304+
* Verifies if the given error is retryable.
305+
*
306+
* @public
307+
* @param {object|undefined|null} error the error object
308+
* @returns {boolean} true if the error is retryable
309+
*/
310+
const isRetryableError = Neo4jError.isRetryable
311+
272312
/**
273313
* @private
274314
* @param {string} code the error code
275315
* @returns {boolean} true if the error is a retriable error
276316
*/
277-
function _isRetriableCode (code?: Neo4jErrorCode): boolean {
317+
function _isRetryableCode (code?: Neo4jErrorCode): boolean {
278318
return code === SERVICE_UNAVAILABLE ||
279319
code === SESSION_EXPIRED ||
280320
_isAuthorizationExpired(code) ||
@@ -313,6 +353,7 @@ export {
313353
newError,
314354
newGQLError,
315355
isRetriableError,
356+
isRetryableError,
316357
Neo4jError,
317358
GQLError,
318359
SERVICE_UNAVAILABLE,

‎packages/core/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
Neo4jError,
2121
newGQLError,
2222
GQLError,
23+
isRetryableError,
2324
isRetriableError,
2425
PROTOCOL_ERROR,
2526
SERVICE_UNAVAILABLE,
@@ -121,6 +122,7 @@ const forExport = {
121122
Neo4jError,
122123
newGQLError,
123124
GQLError,
125+
isRetryableError,
124126
isRetriableError,
125127
error,
126128
Integer,
@@ -196,6 +198,7 @@ export {
196198
Neo4jError,
197199
newGQLError,
198200
GQLError,
201+
isRetryableError,
199202
isRetriableError,
200203
error,
201204
Integer,

‎packages/core/src/internal/transaction-executor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717
/* eslint-disable @typescript-eslint/promise-function-async */
18-
import { newError, isRetriableError } from '../error'
18+
import { newError, isRetryableError } from '../error'
1919
import Transaction, { NonAutoCommitApiTelemetryConfig, NonAutoCommitTelemetryApis } from '../transaction'
2020
import TransactionPromise from '../transaction-promise'
2121
import { TELEMETRY_APIS } from './constants'
@@ -151,7 +151,7 @@ export class TransactionExecutor {
151151
): Promise<T> {
152152
const elapsedTimeMs = Date.now() - retryStartTime
153153

154-
if (elapsedTimeMs > this._maxRetryTimeMs || !isRetriableError(error)) {
154+
if (elapsedTimeMs > this._maxRetryTimeMs || !isRetryableError(error)) {
155155
return Promise.reject(error)
156156
}
157157

‎packages/core/test/__snapshots__/json.test.ts.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,11 @@ exports[`json .stringify should handle object with custom toString in list 1`] =
102102

103103
exports[`json .stringify should handle object with custom toString in object 1`] = `"{"key":{"identity":"1"}}"`;
104104

105-
exports[`json .stringify should handle objects created with createBrokenObject 1`] = `"{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}"`;
105+
exports[`json .stringify should handle objects created with createBrokenObject 1`] = `"{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}"`;
106106

107-
exports[`json .stringify should handle objects created with createBrokenObject in list 1`] = `"[{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}]"`;
107+
exports[`json .stringify should handle objects created with createBrokenObject in list 1`] = `"[{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}]"`;
108108

109-
exports[`json .stringify should handle objects created with createBrokenObject inside other object 1`] = `"{"number":1,"broken":{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}}"`;
109+
exports[`json .stringify should handle objects created with createBrokenObject inside other object 1`] = `"{"number":1,"broken":{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}}"`;
110110

111111
exports[`json .stringify should handle string 1`] = `""my string""`;
112112

‎packages/core/test/error.test.ts

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717
import {
1818
Neo4jError,
19-
isRetriableError,
19+
isRetryableError,
2020
newError,
2121
newGQLError,
2222
PROTOCOL_ERROR,
@@ -123,13 +123,13 @@ describe('newError', () => {
123123
})
124124
})
125125

126-
describe('isRetriableError()', () => {
127-
it.each(getRetriableErrorsFixture())('should return true for error with code %s', error => {
128-
expect(isRetriableError(error)).toBe(true)
126+
describe('isRetryableError()', () => {
127+
it.each(getRetryableErrorsFixture())('should return true for error with code %s', error => {
128+
expect(isRetryableError(error)).toBe(true)
129129
})
130130

131-
it.each(getNonRetriableErrorsFixture())('should return false for error with code %s', error => {
132-
expect(isRetriableError(error)).toBe(false)
131+
it.each(getNonRetryableErrorsFixture())('should return false for error with code %s', error => {
132+
expect(isRetryableError(error)).toBe(false)
133133
})
134134
})
135135

@@ -178,45 +178,47 @@ describe('Neo4jError', () => {
178178
expect(error.constructor).toEqual(Neo4jError)
179179
})
180180

181-
test.each(getRetriableCodes())('should define retriable as true for error with code %s', code => {
181+
test.each(getRetryableCodes())('should define retryable as true for error with code %s', code => {
182182
const error = new Neo4jError('message', code, 'gqlStatus', 'gqlStatusDescription')
183183

184184
expect(error.retriable).toBe(true)
185+
expect(error.retryable).toBe(true)
185186
})
186187

187-
test.each(getNonRetriableCodes())('should define retriable as false for error with code %s', code => {
188+
test.each(getNonRetryableCodes())('should define retryable as false for error with code %s', code => {
188189
const error = new Neo4jError('message', code, 'gqlStatus', 'gqlStatusDescription')
189190

190191
expect(error.retriable).toBe(false)
192+
expect(error.retryable).toBe(false)
191193
})
192194

193-
describe('.isRetriable()', () => {
194-
it.each(getRetriableErrorsFixture())('should return true for error with code %s', error => {
195-
expect(Neo4jError.isRetriable(error)).toBe(true)
195+
describe('.isRetryable()', () => {
196+
it.each(getRetryableErrorsFixture())('should return true for error with code %s', error => {
197+
expect(Neo4jError.isRetryable(error)).toBe(true)
196198
})
197199

198-
it.each(getNonRetriableErrorsFixture())('should return false for error with code %s', error => {
199-
expect(Neo4jError.isRetriable(error)).toBe(false)
200+
it.each(getNonRetryableErrorsFixture())('should return false for error with code %s', error => {
201+
expect(Neo4jError.isRetryable(error)).toBe(false)
200202
})
201203
})
202204
})
203205

204-
function getRetriableErrorsFixture (): Array<[Neo4jError]> {
205-
return getRetriableCodes().map(code => [newError('message', code)])
206+
function getRetryableErrorsFixture (): Array<[Neo4jError]> {
207+
return getRetryableCodes().map(code => [newError('message', code)])
206208
}
207209

208-
function getNonRetriableErrorsFixture (): any[] {
210+
function getNonRetryableErrorsFixture (): any[] {
209211
return [
210212
null,
211213
undefined,
212214
'',
213215
'Neo.TransientError.Transaction.DeadlockDetected',
214216
new Error('Neo.ClientError.Security.AuthorizationExpired'),
215-
...getNonRetriableCodes().map(code => [newError('message', code)])
217+
...getNonRetryableCodes().map(code => [newError('message', code)])
216218
]
217219
}
218220

219-
function getRetriableCodes (): string[] {
221+
function getRetryableCodes (): string[] {
220222
return [
221223
SERVICE_UNAVAILABLE,
222224
SESSION_EXPIRED,
@@ -228,7 +230,7 @@ function getRetriableCodes (): string[] {
228230
]
229231
}
230232

231-
function getNonRetriableCodes (): string[] {
233+
function getNonRetryableCodes (): string[] {
232234
return [
233235
'Neo.DatabaseError.General.UnknownError',
234236
'Neo.DatabaseError.General.OutOfMemoryError',

0 commit comments

Comments
 (0)