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

feat: separate unexpected from expected exceptions #1744

Merged
merged 5 commits into from
Jul 28, 2022
Merged

feat: separate unexpected from expected exceptions #1744

merged 5 commits into from
Jul 28, 2022

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jul 26, 2022

What this PR changes/adds

Distinct between expected (subclasses of EdcApiException) from unexpected (any other) exceptions.
The latter, as unexpected, will return a 5xx error, by default a 500 Internal Server Error, as it's a generic "server failure" error.

Why it does that

To improve error handling.

Further notes

  • introduced a InvalidRequestException that replaces all the IllegalArgumentException in the data-management api modules
  • separated the exception mapper in 3 parts: EdcApiExceptionMapper, ValidationExceptionMapper and UnexpectedExceptionMapper

Linked Issue(s)

Closes #1730

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@ndr-brt ndr-brt added the enhancement New feature or request label Jul 26, 2022
@ndr-brt ndr-brt changed the title api: separate unexpected from expected exceptions feat: separate unexpected from expected exceptions Jul 26, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2022

Codecov Report

Merging #1744 (86e7a95) into main (7b204aa) will increase coverage by 0.03%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main    #1744      +/-   ##
==========================================
+ Coverage   67.33%   67.37%   +0.03%     
==========================================
  Files         785      781       -4     
  Lines       16858    16749     -109     
  Branches     1061     1056       -5     
==========================================
- Hits        11352    11284      -68     
+ Misses       5050     5007      -43     
- Partials      456      458       +2     
Impacted Files Coverage Δ
...r/core/defaults/assetindex/InMemoryAssetIndex.java 92.72% <ø> (+1.49%) ⬆️
...e/dataspaceconnector/api/ServiceResultHandler.java 0.00% <0.00%> (ø)
...ector/api/datamanagement/asset/model/AssetDto.java 88.88% <0.00%> (-11.12%) ⬇️
.../api/datamanagement/asset/model/AssetEntryDto.java 100.00% <ø> (ø)
...api/datamanagement/asset/model/DataAddressDto.java 88.88% <0.00%> (-11.12%) ⬇️
...i/datamanagement/catalog/CatalogApiController.java 75.00% <0.00%> (+3.57%) ⬆️
...actnegotiation/model/ContractOfferDescription.java 100.00% <ø> (ø)
...nsion/jersey/mapper/ValidationExceptionMapper.java 100.00% <ø> (ø)
.../dataspaceconnector/spi/result/AbstractResult.java 90.00% <ø> (ø)
...p/webhook/HttpProvisionerWebhookApiController.java 95.83% <87.50%> (-4.17%) ⬇️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b204aa...86e7a95. Read the comment docs.

@ndr-brt ndr-brt requested a review from jimmarino as a code owner July 26, 2022 13:45
Copy link
Member

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

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

comments inline

@ndr-brt ndr-brt merged commit 813a6e7 into eclipse-edc:main Jul 28, 2022
@ndr-brt ndr-brt deleted the feature/1730-separate-expected-from-unexpected-exceptions branch July 28, 2022 06:24
diegogomez-zf pushed a commit to diegogomez-zf/DataSpaceConnector that referenced this pull request Aug 3, 2022
* api: separate unexpected from expected exceptions

* Fix tests

* PR remark

* Simplify validation

* Improve Result to List<ApiErrorDetail> mapping by carrying failure messages through the EdcApiException
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

api: clearly separate "expected" exceptions from "unexpected" ones
3 participants