Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): error handling improved #613

Merged
merged 9 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@
},
"dependencies": {
"@amplitude/analytics-browser": "^0.3.1",
"@hawk.so/javascript": "^3.0.0",
"@hawk.so/javascript": "^3.0.6",
"@hawk.so/types": "^0.1.18",
"@hawk.so/webpack-plugin": "^1.0.1",
"@vue/cli-plugin-babel": "^4.1.2",
"@vue/cli-plugin-pwa": "^4.1.2",
"@vue/cli-plugin-typescript": "^4.1.2",
"@vue/cli-service": "^4.5.15",
"axios": "^0.25.0",
Expand All @@ -40,7 +39,6 @@
"postcss-nested-ancestors": "^2.0.0",
"postcss-preset-env": "^6.6.0",
"postcss-simple-vars": "^5.0.2",
"register-service-worker": "^1.6.2",
"short-number": "^1.0.7",
"spa-http-server": "^0.9.0",
"svgo": "^1.1.0",
Expand Down
41 changes: 0 additions & 41 deletions public/service-worker.js

This file was deleted.

92 changes: 65 additions & 27 deletions src/api/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axios, { AxiosResponse } from 'axios';
import { prepareFormData } from '@/api/utils';
import { APIResponse } from '../types/api';
import { useErrorTracker } from '@/hawk';

/**
* Hawk API endpoint URL
Expand All @@ -17,6 +18,11 @@ let blockingRequest: Promise<AxiosResponse>;
*/
let tokenRefreshingRequest: Promise<string> | null;

/**
* Error tracking composable
*/
const { track } = useErrorTracker();

/**
* Describe format of the GraphQL API error item
*/
Expand Down Expand Up @@ -129,34 +135,56 @@ export async function callOld(

let response;

if (initial || force) {
response = await promise;
} else {
response = (await Promise.all([blockingRequest, promise]))[1];
}
try { // handle axios errors
if (initial || force) {
response = await promise;
} else {
response = (await Promise.all([blockingRequest, promise]))[1];
}

if (response.data.errors) {
response.data.errors.forEach(error => {
printApiError(error, response.data, request, variables);
});
}

/**
* For now (Apr 10, 2020) all previous code await to get only data
* so new request will pass allowErrors=true and get both errors and data
*
* @todo refactor old requests same way
*/
if (allowErrors) {
return response.data;
} else {
// console.warn('Api call in old format. Should be refactored to support errors', request);
}
if (response.data.errors) {
response.data.errors.forEach(error => {
/**
* Send error to Hawk
*/
track(new Error(error.message), {
'Request': request,
'Error Path': error.path,
'Variables': variables ?? {},
'Response Data': response.data.data,
});

printApiError(error, response.data, request, variables);
});
}

/**
* @deprecated old format. See method jsdoc
*/
return response.data.data;
/**
* For now (Apr 10, 2020) all previous code await to get only data
* so new request will pass allowErrors=true and get both errors and data
*
* @todo refactor old requests same way
*/
if (allowErrors) {
return response.data;
} else {
// console.warn('Api call in old format. Should be refactored to support errors', request);
}

/**
* @deprecated old format. See method jsdoc
*/
return response.data.data;
} catch (error) {
console.error('API Request Error', error);

track(error as Error, {
'Request': request,
'Variables': variables ?? {},
'Response Data': response?.data.data,
});
throw error;
}
}

/**
Expand All @@ -173,7 +201,7 @@ export async function call(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
variables?: Record<string, any>,
files?: {[name: string]: File | undefined},
{ initial = false, force = false }: ApiCallSettings = {}
{ initial = false, force = false, allowErrors = false }: ApiCallSettings = {}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
): Promise<APIResponse<any>> {
const response = await callOld(request, variables, files, Object.assign({
Expand All @@ -186,8 +214,18 @@ export async function call(
/**
* Response can contain errors.
* Throw such errors to the Vue component to display them for user
*
* Note from 2024-04-10:
* - Now we have try-catch block components, since errors are thrown from the API module
* - But it would be more safe to not throw errors from the API module, but return them in the response and then handle them in store or component.
* - Refactoring steps: (@todo)
* 1. Rewrire all requests to use api.call() instead of api.callOld()
* 2. Get rid of allowErrors flag form the api.callOld() method
* 3. Provide a way to handle errors in the store
* 4. Review all try/catch statements in the components and remove them
* - For a temporary solution, we explicitly pass allowErrors=true when method is ready to receive errors as well as data
*/
if (response.errors && response.errors.length) {
if (response.errors && response.errors.length && allowErrors === false) {
response.errors.forEach(error => {
throw new Error(error.message);
});
Expand Down
6 changes: 6 additions & 0 deletions src/api/workspaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export async function leaveWorkspace(workspaceId: string): Promise<boolean> {
export async function getAllWorkspacesWithProjects(): Promise<APIResponse<{ workspaces: Workspace[] }>> {
return api.call(QUERY_ALL_WORKSPACES_WITH_PROJECTS, undefined, undefined, {
initial: true,

/**
* This request calls on the app start, so we don't want to break app if something goes wrong
* With this flag, errors from the API won't be thrown, but returned in the response for further handling
*/
allowErrors: true,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/api/workspaces/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
* Query for getting all user's workspaces and project.
*/
export const QUERY_ALL_WORKSPACES_WITH_PROJECTS = `
{
query AllWorkspacesWithProjects {
workspaces {
id
name
Expand Down
47 changes: 26 additions & 21 deletions src/components/AppShell.vue
Original file line number Diff line number Diff line change
Expand Up @@ -205,32 +205,37 @@ export default {
* Vue hook. Called synchronously after the instance is created
*/
async created() {
/**
* Fetch user data
*/
await this.$store.dispatch(FETCH_INITIAL_DATA);
try {
/**
* Fetch user data
*/
await this.$store.dispatch(FETCH_INITIAL_DATA);

this.$store.dispatch(RESET_MODAL_DIALOG);
this.$store.dispatch(RESET_MODAL_DIALOG);

/**
* Onboarding. If a user has no workspace, show Create Workspace modal
*/
this.suggestWorkspaceCreation();
/**
* Onboarding. If a user has no workspace, show Create Workspace modal
*/
this.suggestWorkspaceCreation();

/**
* Get current workspace
*/
const workspace = this.$store.getters.getWorkspaceById(this.workspaceId);
/**
* Get current workspace
*/
const workspace = this.$store.getters.getWorkspaceById(this.workspaceId);

/**
* Set current workspace
*/
this.$store.dispatch(SET_CURRENT_WORKSPACE, workspace);
/**
* Set current workspace
*/
this.$store.dispatch(SET_CURRENT_WORKSPACE, workspace);

/**
* Fetch current user data
*/
this.$store.dispatch(FETCH_CURRENT_USER);
/**
* Fetch current user data
*/
this.$store.dispatch(FETCH_CURRENT_USER);
} catch (error) {
console.error(error);
this.$sendToHawk(`Error on app initialization!: ${error.message}`);
}
},
methods: {
onModalClose() {
Expand Down
75 changes: 75 additions & 0 deletions src/hawk.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import HawkCatcher, { HawkInitialSettings, HawkJavaScriptEvent } from '@hawk.so/javascript'
import type Vue from 'vue';

/**
* Current build revision
* passed from Webpack Define Plugin
*/
declare const buildRevision: string;

/**
* Initial options of error tracking composable
*/
export interface ErrorTrackerInitialOptions {
/**
* Instance of the Vue app
*/
vue: typeof Vue;

/**
* Current user to be attached to events
*/
user?: HawkJavaScriptEvent['user'];
}

/**
* Shared instance of Hawk.so
* null if Hawk is not initialized
*/
let hawk: HawkCatcher | null = null;

/**
* Composable for tracking errors via Hawk.so
*/
export function useErrorTracker(): {
init: (options: ErrorTrackerInitialOptions) => void;
track: (...args: Parameters<HawkCatcher['send']>) => void;
} {
/**
* Initialize Hawk.so
*
* @param options - params to be passed to hawk initialization
*/
function init({ vue, user }: ErrorTrackerInitialOptions): void {
if (process.env.VUE_APP_HAWK_TOKEN) {
const hawkOptions: HawkInitialSettings = {
token: process.env.VUE_APP_HAWK_TOKEN,
release: buildRevision,
vue,
};

if (user) {
hawkOptions.user = user;
}

hawk = new HawkCatcher(hawkOptions);
}
}

/**
* Method for manual error sending
*
* @param error - error to track
* @param context - additional context
*/
function track(...args: Parameters<HawkCatcher['send']>): void {
if (hawk) {
hawk.send(...args);
}
}

return {
init,
track,
};
}
Loading
Loading