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

Safely access caught Error properties #3196

Merged
merged 2 commits into from
Sep 8, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ export default function DescriptionField({
await onUpdate(desc || '');
// message.destroy();
// message.success({ content: 'Updated!', duration: 2 });
} catch (e) {
} catch (e: unknown) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there cases where "e" is not an Error? I have not encountered them but interested to hear if you've seen this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the original proposal. It tries to be more specific on the error types, like other languages. Like the example from that thread

try {
  externalApi()
} catch(e) {
  if (e instanceof Error) {
    // Probably send this error to developer of API
  } else if (e instanceof ApiError) {
    // We messed up, log it and fix it in next patch
  }
}

message.destroy();
message.error({ content: `Update Failed! \n ${e.message || ''}`, duration: 2 });
if (e instanceof Error) message.error({ content: `Update Failed! \n ${e.message || ''}`, duration: 2 });
}
onCloseModal();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export default function UpdatableDescription({
entityUrn: urn,
});
message.success({ content: 'Updated!', duration: 2 });
} catch (e) {
} catch (e: unknown) {
message.destroy();
message.error({ content: `Update Failed! \n ${e.message || ''}`, duration: 2 });
if (e instanceof Error) message.error({ content: `Update Failed! \n ${e.message || ''}`, duration: 2 });
}
setShowAddDescModal(false);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ export const AddLinkModal = ({ buttonProps }: { buttonProps?: Record<string, unk
variables: { input: { urn, institutionalMemory: { elements: newLinks } } },
});
message.success({ content: 'Link Added', duration: 2 });
} catch (e) {
} catch (e: unknown) {
message.destroy();
message.error({ content: `Failed to add link: \n ${e.message || ''}`, duration: 3 });
if (e instanceof Error) {
message.error({ content: `Failed to add link: \n ${e.message || ''}`, duration: 3 });
}
}

handleClose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ export const DescriptionEditor = ({ onComplete }: { onComplete?: () => void }) =
});
message.success({ content: 'Description Updated', duration: 2 });
if (onComplete) onComplete();
} catch (e) {
} catch (e: unknown) {
message.destroy();
message.error({ content: `Failed to update description: \n ${e.message || ''}`, duration: 2 });
if (e instanceof Error) {
message.error({ content: `Failed to update description: \n ${e.message || ''}`, duration: 2 });
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ export const LinkList = () => {
variables: { input: { urn, institutionalMemory: { elements: newLinks } } },
});
message.success({ content: 'Link Deleted', duration: 2 });
} catch (e) {
} catch (e: unknown) {
message.destroy();
message.error({ content: `Error deleting link: \n ${e.message || ''}`, duration: 2 });
if (e instanceof Error) {
message.error({ content: `Error deleting link: \n ${e.message || ''}`, duration: 2 });
}
}
};

Expand Down