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

chore: improve joi errors #3836

Merged
merged 5 commits into from
Jun 7, 2023
Merged

chore: improve joi errors #3836

merged 5 commits into from
Jun 7, 2023

Conversation

thomasheartman
Copy link
Contributor

@thomasheartman thomasheartman commented May 23, 2023

This PR improves our handling of internal Joi errors, to make them more sensible to the end user. It does that by providing a better description of the errors and by telling the user what they value they provided was.

Previous conversion:

{
  "id": "705a8dc0-1198-4894-9015-f1e5b9992b48",
  "name": "BadDataError",
  "message": "\"value\" must contain at least 1 items",
  "details": [
    {
      "message": "\"value\" must contain at least 1 items",
      "description": "\"value\" must contain at least 1 items"
    }
  ]
}

New conversion:

{
  "id": "87fb4715-cbdd-48bb-b4d7-d354e7d97380",
  "name": "BadDataError",
  "message": "Request validation failed: your request body contains invalid data. Refer to the `details` list for more information.",
  "details": [
    {
      "description": "\"value\" must contain at least 1 items. You provided [].",
      "message": "\"value\" must contain at least 1 items. You provided []."
    }
  ]
}

Restructuring

This PR moves some code out of unleash-error.ts and into a new file. The purpose of this is twofold:

  1. avoid circular dependencies (we need imports from both UnleashError and BadDataError)
  2. carve out a clearer separation of concerns, keeping unleash-error a little more focused.

@vercel
Copy link

vercel bot commented May 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Jun 7, 2023 8:21am
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Jun 7, 2023 8:21am

};
});

const [first, ...rest] = details;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this and check for first afterwards? 🤔
Couldn't we check details.length on line 140 and pass in details in the second parameter in 143?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! I've tried that multiple times, but typescript doesn't accept that, I'm afraid 🤷🏼

Copy link
Member

@nunogois nunogois left a comment

Choose a reason for hiding this comment

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

It seems like we may need to update some of the existing tests to fix them, but this looks good to me!

@thomasheartman
Copy link
Contributor Author

Yeah, you're right. Those errors also provide more details now, so I've updated the tests in 30f007e. Thanks for the review 🙏🏼

@thomasheartman thomasheartman enabled auto-merge (squash) June 7, 2023 08:21
@thomasheartman thomasheartman merged commit 24aea5f into main Jun 7, 2023
9 checks passed
@thomasheartman thomasheartman deleted the chore/improve-joi-errors branch June 7, 2023 08:29
thomasheartman added a commit that referenced this pull request Jun 8, 2023
This PR improves our handling of internal Joi errors, to make them more
sensible to the end user. It does that by providing a better description
of the errors and by telling the user what they value they provided was.

Previous conversion:
```json
{
  "id": "705a8dc0-1198-4894-9015-f1e5b9992b48",
  "name": "BadDataError",
  "message": "\"value\" must contain at least 1 items",
  "details": [
    {
      "message": "\"value\" must contain at least 1 items",
      "description": "\"value\" must contain at least 1 items"
    }
  ]
}
```

New conversion:
```json
{
  "id": "87fb4715-cbdd-48bb-b4d7-d354e7d97380",
  "name": "BadDataError",
  "message": "Request validation failed: your request body contains invalid data. Refer to the `details` list for more information.",
  "details": [
    {
      "description": "\"value\" must contain at least 1 items. You provided [].",
      "message": "\"value\" must contain at least 1 items. You provided []."
    }
  ]
}
```

## Restructuring

This PR moves some code out of `unleash-error.ts` and into a new file.
The purpose of this is twofold:
1. avoid circular dependencies (we need imports from both UnleashError
and BadDataError)
2. carve out a clearer separation of concerns, keeping `unleash-error` a
little more focused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants