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

[connectors] Fix(Zendesk) - users not found #8912

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Conversation

aubin-tchoi
Copy link
Contributor

Description

  • Fix issues similar to these.
  • The response most likely contains an object with error details instead of an array.
  • Since this only occurred once, this will be logged and set aside for now.

Risk

Low.

Deploy Plan

  • Deploy connectors.

@@ -427,7 +427,7 @@ export async function syncZendeskTicketBatchActivity({
return { hasMore: false, nextLink: "" };
}

const users = await zendeskApiClient.users.list();
const users = (await zendeskApiClient.users.list()) || [];
Copy link
Contributor

Choose a reason for hiding this comment

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

can .list() return undefined ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not in the types, yes in the actual implementation

Copy link
Contributor

@albandum albandum left a comment

Choose a reason for hiding this comment

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

I think we're not fond of trycatch here :P

Comment on lines +178 to +187
let author;
try {
author = users.find((user) => user.id === comment.author_id);
} catch (e) {
logger.warn(
{ connectorId, e, ...loggerArgs },
"[Zendesk] Error finding the author of a comment."
);
author = null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let author;
try {
author = users.find((user) => user.id === comment.author_id);
} catch (e) {
logger.warn(
{ connectorId, e, ...loggerArgs },
"[Zendesk] Error finding the author of a comment."
);
author = null;
}
const author = users?.find((user) => user?.id === comment?.author_id) || null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's not really what we're trying to fix with our try catch, will safely ignore for now

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah actually not sure what this is supposed to do ?

Copy link
Contributor

Choose a reason for hiding this comment

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

if there's an error here then it's a typescript error no ? there's no async at all

@aubin-tchoi aubin-tchoi merged commit 635eafc into main Nov 26, 2024
3 checks passed
@aubin-tchoi aubin-tchoi deleted the zendesk/fix-user-find branch November 26, 2024 15:27
overmode pushed a commit that referenced this pull request Nov 27, 2024
* fix: fix issues with users not found by the SDK

* fix: ignore issues with users not found by the SDK

* 🔊
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants