Skip to content

Commit a881987

Browse files
[Kernel] Add exception principles for Kernel to solidify the rules for our exception framework (#3408)
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [ ] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description <!-- - Describe what this PR changes. - Describe why we need the change. If this PR resolves an issue be sure to include "Resolves #XXX" to correctly link and close the issue upon merge. --> ## How was this patch tested? <!-- If tests were added, say they were added here. Please make sure to test the changes thoroughly including negative and positive cases if possible. If the changes were tested in any way other than unit tests, please clarify how you tested step by step (ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future). If the changes were not tested, please explain why. --> ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. -->
1 parent cfd9133 commit a881987

File tree

1 file changed

+42
-0
lines changed

1 file changed

+42
-0
lines changed

kernel/EXCEPTION_PRINCIPLES.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Exception principles in Delta Kernel
2+
## Introduction
3+
Exceptions thrown in Delta Kernel are either user-facing or developer-facing.
4+
- **User-facing exceptions** are expected to be thrown. Delta Kernel is unable to complete the requested operation for a fundamental reason inherent to the nature of the request, the table of interest, and the capabilities of Delta Kernel and the Delta protocol. These errors are intentional and are used to communicate with the end-user why an operation cannot be completed.
5+
- **Developer-facing exceptions** are unexpected and generally indicate that something has gone wrong or is incorrect. They can target either Kernel developers or connector developers that are using Kernel APIs. These exceptions should be used for debugging; a perfectly working connector + Kernel should never encounter these.
6+
7+
See [User-facing vs developer-facing exceptions](#User-facing-vs-developer-facing-exceptions) for examples of these types of exceptions.
8+
9+
## Principles
10+
These are the general exception principles to follow and enforce when contributing code or reviewing pull requests.
11+
- All **user-facing exceptions** should be of type `KernelException`.
12+
- Create a new subclass for exceptions that may require special handling (such as `TableNotFoundException`) otherwise just use `KernelException`. Subclasses should expose useful exception parameters on a case-by-case basis.
13+
- All `KernelException`s should be instantiated with a method in the [DeltaErrors](https://github.com/delta-io/delta/blob/master/kernel/kernel-api/src/main/java/io/delta/kernel/internal/DeltaErrors.java) file.
14+
- Error messages should be clear and actionable.
15+
- Clearly state (1) the problem, (2) why it occurred and (3) how it can be solved.
16+
- **User-facing exceptions** should be consistent across releases. Any changes to user-facing exception classes or messages should be carefully reviewed.
17+
- Any unchecked exceptions originating from the `Engine` implementation should be wrapped with `KernelEngineException` and should include additional context about the failing operation.
18+
- This means all method calls to the `Engine` implementation should be wrapped. See [Wrapping exceptions thrown from the Engine implementation](#Wrapping-exceptions-thrown-from-the-Engine-implementation) for more details.
19+
- **Developer-facing exceptions** should be informative and provide useful information for debugging.
20+
21+
## Further details
22+
23+
### User-facing vs developer-facing exceptions
24+
25+
User-facing exceptions:
26+
- `TableNotFoundException` when there is no Delta table at the provided path.
27+
- Reading the Change Data Feed from a table without CDF enabled.
28+
- The input data violates table constraints when writing to the table.
29+
- Kernel doesn’t support reading a table with XXX table feature.
30+
31+
Developer-facing exceptions:
32+
- `getInt` is called on a boolean `ColumnVector`.
33+
- A column mapping mode besides “none”, “id”, and “name” is encountered.
34+
- An empty iterator is returned from the `Engine` implementation when reading files.
35+
36+
### Wrapping exceptions thrown from the Engine implementation
37+
We want to wrap any unchecked exceptions thrown from the `Engine` implementation with `KernelEngineException` and include additional context about the failing operation. This makes it clear where the exception is originating from, and the additional context can help future debugging.
38+
39+
This requires wrapping all method calls into the `Engine` implementation. We do this using helper methods in `DeltaErrors` like [wrapEngineException](https://github.com/delta-io/delta/blob/4fefba182f81d39f1d11e2f2b85bfa140079ea11/kernel/kernel-api/src/main/java/io/delta/kernel/internal/DeltaErrors.java#L228-L240). For usage see [example 1](https://github.com/delta-io/delta/blob/2b2ef732533c707b7ca1af30e2a059da86c3c3ff/kernel/kernel-api/src/main/java/io/delta/kernel/internal/TransactionImpl.java#L246-L256) and [example 2](https://github.com/delta-io/delta/blob/2b2ef732533c707b7ca1af30e2a059da86c3c3ff/kernel/kernel-api/src/main/java/io/delta/kernel/internal/ScanImpl.java#L236-L244).
40+
41+
Note: this does not catch all exceptions originating from the engine implementation, as exceptions that are not thrown until access will not be wrapped (i.e. exceptions thrown within iterators, in `ColumnVector` implementations, etc)
42+
- When checked exceptions cannot be thrown we instead wrap the checked exception in a `KernelEngineException`. See [here](https://github.com/delta-io/delta/blob/2b2ef732533c707b7ca1af30e2a059da86c3c3ff/kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/parquet/ParquetFileReader.java#L148-L150) for an example.

0 commit comments

Comments
 (0)