Skip to content

Commit

Permalink
feat: rework error handler (#9861)
Browse files Browse the repository at this point in the history 
* feat: add new error handler

* feat: remove error handler return type

* Update packages/datastore/src/sync/processors/sync.ts

Co-authored-by: Jon Wire <iambipedal@gmail.com>

* Update packages/datastore/src/sync/processors/mutation.ts

Co-authored-by: Jon Wire <iambipedal@gmail.com>

* Update packages/datastore/src/sync/processors/subscription.ts

Co-authored-by: Jon Wire <iambipedal@gmail.com>

* Update packages/datastore/src/types.ts

Co-authored-by: Jon Wire <iambipedal@gmail.com>

* Update packages/datastore/src/sync/utils.ts

Co-authored-by: Jon Wire <iambipedal@gmail.com>

* style: move error map

* fix: move subscription error handler up

* style: fix tslint

* fix: typo

* fix: make error handler required in sync processors

Co-authored-by: ArkamJ <arkamj@amazon.com>
Co-authored-by: Jon Wire <iambipedal@gmail.com>
  • Loading branch information
@dpilch @ArkamJ @svidgen
3 people authored May 3, 2022
1 parent 98b6e11 commit 6ae8d10
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 30 deletions.
1 change: 1 addition & 0 deletions packages/datastore/__tests__/mutation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ async function instantiateMutationProcessor() {
aws_appsync_authenticationType: 'API_KEY',
aws_appsync_apiKey: 'da2-xxxxxxxxxxxxxxxxxxxxxx',
},
() => null,
() => null
);

Expand Down
4 changes: 2 additions & 2 deletions packages/datastore/src/datastore/datastore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ function defaultConflictHandler(conflictData: SyncConflict): PersistentModel {
return modelInstanceCreator(modelConstructor, { ...localModel, _version });
}

function defaultErrorHandler(error: SyncError) {
function defaultErrorHandler(error: SyncError<PersistentModel>): void {
logger.warn(error);
}

Expand Down Expand Up @@ -686,7 +686,7 @@ class DataStore {
private amplifyConfig: Record<string, any> = {};
private authModeStrategy: AuthModeStrategy;
private conflictHandler: ConflictHandler;
private errorHandler: (error: SyncError) => void;
private errorHandler: (error: SyncError<PersistentModel>) => void;
private fullSyncInterval: number;
private initialized: Promise<void>;
private initReject: Function;
Expand Down
10 changes: 6 additions & 4 deletions packages/datastore/src/sync/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ export class SyncEngine {
this.schema,
this.syncPredicates,
this.amplifyConfig,
this.authModeStrategy
this.authModeStrategy,
errorHandler
);
this.subscriptionsProcessor = new SubscriptionProcessor(
this.schema,
this.syncPredicates,
this.amplifyConfig,
this.authModeStrategy
this.authModeStrategy,
errorHandler
);
this.mutationsProcessor = new MutationProcessor(
this.schema,
Expand All @@ -153,8 +155,8 @@ export class SyncEngine {
MutationEvent,
this.amplifyConfig,
this.authModeStrategy,
conflictHandler,
errorHandler
errorHandler,
conflictHandler
);
this.datastoreConnectivity = new DataStoreConnectivity();
}
Expand Down
25 changes: 17 additions & 8 deletions packages/datastore/src/sync/processors/mutation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
PersistentModelConstructor,
SchemaModel,
TypeConstructorMap,
ProcessName,
} from '../../types';
import { exhaustiveCheck, USER } from '../../util';
import { MutationEventOutbox } from '../outbox';
Expand All @@ -33,12 +34,19 @@ import {
getModelAuthModes,
TransformerMutationType,
getTokenForCustomAuth,
ErrorMap,
mapErrorToType,
} from '../utils';

const MAX_ATTEMPTS = 10;

const logger = new Logger('DataStore');

// TODO: add additional error maps
const errorMap = {
BadRecord: error => /^Cannot return \w+ for [\w-_]+ type/.test(error.message),
} as ErrorMap;

type MutationProcessorEvent = {
operation: TransformerMutationType;
modelDefinition: SchemaModel;
Expand All @@ -63,8 +71,8 @@ class MutationProcessor {
private readonly MutationEvent: PersistentModelConstructor<MutationEvent>,
private readonly amplifyConfig: Record<string, any> = {},
private readonly authModeStrategy: AuthModeStrategy,
private readonly conflictHandler?: ConflictHandler,
private readonly errorHandler?: ErrorHandler
private readonly errorHandler: ErrorHandler,
private readonly conflictHandler?: ConflictHandler
) {
this.generateQueries();
}
Expand Down Expand Up @@ -373,20 +381,21 @@ class MutationProcessor {
} else {
try {
await this.errorHandler({
localModel: this.modelInstanceCreator(
modelConstructor,
variables.input
),
recoverySuggestion:
'Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues',
localModel: variables.input,
message: error.message,
operation,
errorType: error.errorType,
errorType: mapErrorToType(errorMap, error),
errorInfo: error.errorInfo,
process: ProcessName.mutate,
cause: error,
remoteModel: error.data
? this.modelInstanceCreator(modelConstructor, error.data)
: null,
});
} catch (err) {
logger.warn('failed to execute errorHandler', err);
logger.warn('Mutation error handler failed with:', err);
} finally {
// Return empty tuple, dequeues the mutation
return error.data
Expand Down
43 changes: 39 additions & 4 deletions packages/datastore/src/sync/processors/subscription.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
PredicatesGroup,
ModelPredicate,
AuthModeStrategy,
ErrorHandler,
ProcessName,
} from '../../types';
import {
buildSubscriptionGraphQLOperation,
Expand All @@ -20,12 +22,27 @@ import {
getUserGroupsFromToken,
TransformerMutationType,
getTokenForCustomAuth,
mapErrorToType,
ErrorMap,
} from '../utils';
import { ModelPredicateCreator } from '../../predicates';
import { validatePredicate } from '../../util';

const logger = new Logger('DataStore');

// TODO: add additional error maps
const errorMap = {
Unauthorized: (givenError: any) => {
const {
error: { errors: [{ message = '' } = {}] } = {
errors: [],
},
} = givenError;
const regex = /Connection failed.+Unauthorized/;
return regex.test(message);
},
} as ErrorMap;

export enum CONTROL_MSG {
CONNECTED = 'CONNECTED',
}
Expand Down Expand Up @@ -56,7 +73,8 @@ class SubscriptionProcessor {
private readonly schema: InternalSchema,
private readonly syncPredicates: WeakMap<SchemaModel, ModelPredicate<any>>,
private readonly amplifyConfig: Record<string, any> = {},
private readonly authModeStrategy: AuthModeStrategy
private readonly authModeStrategy: AuthModeStrategy,
private readonly errorHandler: ErrorHandler
) {}

private buildSubscription(
Expand Down Expand Up @@ -436,12 +454,31 @@ class SubscriptionProcessor {
}
this.drainBuffer();
},
error: subscriptionError => {
error: async subscriptionError => {
const {
error: { errors: [{ message = '' } = {}] } = {
errors: [],
},
} = subscriptionError;
try {
await this.errorHandler({
recoverySuggestion:
'Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues',
localModel: null,
message,
model: modelDefinition.name,
operation,
errorType: mapErrorToType(
errorMap,
subscriptionError
),
process: ProcessName.subscribe,
remoteModel: null,
cause: subscriptionError,
});
} catch (e) {
logger.error('Sync error handler failed with:', e);
}

if (
message.includes(
Expand Down Expand Up @@ -487,7 +524,6 @@ class SubscriptionProcessor {
return;
}
}

logger.warn('subscriptionError', message);

if (typeof subscriptionReadyCallback === 'function') {
Expand All @@ -500,7 +536,6 @@ class SubscriptionProcessor {
) {
return;
}

observer.error(message);
},
})
Expand Down
37 changes: 33 additions & 4 deletions packages/datastore/src/sync/processors/sync.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
PredicatesGroup,
GraphQLFilter,
AuthModeStrategy,
ErrorHandler,
ProcessName,
} from '../../types';
import {
buildGraphQLOperation,
Expand All @@ -16,6 +18,8 @@ import {
getForbiddenError,
predicateToGraphQLFilter,
getTokenForCustomAuth,
mapErrorToType,
ErrorMap,
} from '../utils';
import {
jitteredExponentialRetry,
Expand All @@ -24,7 +28,7 @@ import {
NonRetryableError,
} from '@aws-amplify/core';
import { ModelPredicateCreator } from '../../predicates';

import { ModelInstanceCreator } from '../../datastore/datastore';
const opResultDefaults = {
items: [],
nextToken: null,
Expand All @@ -33,14 +37,21 @@ const opResultDefaults = {

const logger = new Logger('DataStore');

// TODO: add additional error maps
const errorMap = {
BadRecord: error => /^Cannot return \w+ for [\w-_]+ type/.test(error.message),
} as ErrorMap;

class SyncProcessor {
private readonly typeQuery = new WeakMap<SchemaModel, [string, string]>();

constructor(
private readonly schema: InternalSchema,
private readonly syncPredicates: WeakMap<SchemaModel, ModelPredicate<any>>,
private readonly amplifyConfig: Record<string, any> = {},
private readonly authModeStrategy: AuthModeStrategy
private readonly authModeStrategy: AuthModeStrategy,
private readonly errorHandler: ErrorHandler,
private readonly modelInstanceCreator?: ModelInstanceCreator
) {
this.generateQueries();
}
Expand Down Expand Up @@ -223,15 +234,33 @@ class SyncProcessor {
error.data[opName] &&
error.data[opName].items
);

if (this.partialDataFeatureFlagEnabled()) {
if (hasItems) {
const result = error;
result.data[opName].items = result.data[opName].items.filter(
item => item !== null
);

if (error.errors) {
await Promise.all(
error.errors.map(async err => {
try {
await this.errorHandler({
recoverySuggestion:
'Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues',
localModel: null,
message: err.message,
model: modelDefinition.name,
operation: opName,
errorType: mapErrorToType(errorMap, err),
process: ProcessName.sync,
remoteModel: null,
cause: err,
});
} catch (e) {
logger.error('Sync error handler failed with:', e);
}
})
);
Hub.dispatch('datastore', {
event: 'syncQueriesPartialSyncError',
data: {
Expand Down
22 changes: 22 additions & 0 deletions packages/datastore/src/sync/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ModelOperation,
InternalSchema,
AuthModeStrategy,
ErrorType,
} from '../types';
import { exhaustiveCheck } from '../util';
import { MutationEvent } from './';
Expand All @@ -40,6 +41,27 @@ enum GraphQLOperationType {
GET = 'query',
}

export type ErrorMap = {
[key in ErrorType]: (error: Error) => boolean;
};

/**
* Categorizes an error with a broad error type, intended to make
* customer error handling code simpler.
* @param errorMap Error names and a list of patterns that indicate them (each pattern as a regex or function)
* @param error The underying error to categorize.
*/
export function mapErrorToType(errorMap: ErrorMap, error: Error): ErrorType {
const errorTypes = [...Object.keys(errorMap)] as ErrorType[];
for (const errorType of errorTypes) {
const matcher = errorMap[errorType];
if (matcher(error)) {
return errorType;
}
}
return 'Unknown';
}

export enum TransformerMutationType {
CREATE = 'Create',
UPDATE = 'Update',
Expand Down
Loading

0 comments on commit 6ae8d10

Please sign in to comment.