Skip to content

Conversation

@JiriLojda
Copy link
Member

Motivation

Which issue does this fix? Fixes #issue number

If no issue exists, what is the fix or new feature? Were there any reasons to fix/implement things that are not obvious?

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

If manual testing is required, what are the steps?

@JiriLojda JiriLojda requested review from a team and IvanKiral as code owners June 16, 2025 08:13
@pokornyd pokornyd self-requested a review June 16, 2025 10:56
return `Response failed with status '${adapterResponse.status}' and status text '${adapterResponse.statusText}'.${kontentErrorResponse.message}${validationErrorMessage ? ` ${validationErrorMessage}` : ""}`;
}

return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

I would probably fallback to some string like "unknown error" or returned the stringified response or something. that way you could simplify the return value of getErrorMessage on line 22.

Copy link
Member

Choose a reason for hiding this comment

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

The unknown error should be reserved for cases which are not accounted for. In this case, I want to give the user a "friendly" error message with status codes & flattened validation error message. This case only happens when the requests itself succeeds, but fails due to Kontent.ai validation.

I did simplify the code here a bit though :)

@pokornyd pokornyd self-requested a review June 17, 2025 08:53
@Enngage Enngage merged commit 9c838d1 into master Jun 17, 2025
3 checks passed
@Enngage Enngage deleted the modernization branch June 17, 2025 09:17
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.

4 participants