Skip to content

Document error handling#1926

Merged
kolyshkin merged 1 commit intocontainers:mainfrom
eriksjolund:document-error-handling
Mar 16, 2026
Merged

Document error handling#1926
kolyshkin merged 1 commit intocontainers:mainfrom
eriksjolund:document-error-handling

Conversation

@eriksjolund
Copy link
Contributor

@eriksjolund eriksjolund commented Dec 12, 2025

Closes: #1860

Summary by Sourcery

Documentation:

  • Add a design document describing standard error handling patterns and expectations for libcrun_error_t-based APIs, including success/failure semantics and known exceptions.

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 12, 2025

Reviewer's Guide

Adds a new design document describing the standard error-handling conventions for C source files under src/, including expected function signatures, success/failure return patterns, and libcrun_error_s usage, with a placeholder section for documented exceptions.

File-Level Changes

Change Details Files
Introduce design documentation for C error-handling conventions using libcrun_error_t and libcrun_error_s.
  • Create a new error_handling.md design document under design-docs/
  • Specify the required function signature pattern using libcrun_error_t *err and int/pointer returns for functions that can fail
  • Define success semantics: return 0 or non-NULL pointer and leave the err argument untouched
  • Define failure semantics: return negative value or non-NULL pointer and set err to an allocated libcrun_error_s with errno-style status and allocated message
  • Clarify that err must not point to an already allocated libcrun_error_s
  • Add a placeholder section to later enumerate exceptions to these conventions
design-docs/error_handling.md

Assessment against linked issues

Issue Objective Addressed Explanation
#1860 Document the error handling convention used in the crun C source code (for functions taking libcrun_error_t *err), including how return values relate to error creation and the expectation that callers do not pass an already-created error object.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@giuseppe
Copy link
Member

Thanks for tackling this one!

Do you think we could also add an AGENTS.md file that links to this file?

@packit-as-a-service
Copy link

TMT tests failed. @containers/packit-build please check.

@eriksjolund eriksjolund force-pushed the document-error-handling branch from 8c03be9 to c789d63 Compare January 9, 2026 07:44
@eriksjolund
Copy link
Contributor Author

Do you think we could also add an AGENTS.md file that links to this file?

Sounds like a good idea.
I see you opened a PR for AGENTS.md

@eriksjolund eriksjolund marked this pull request as ready for review January 9, 2026 07:57
@eriksjolund
Copy link
Contributor Author

I could also that I'm not so experienced in writing this type of documentation. Feel free to close this PR and open another instead. (Probably an AI would do a better job)

@kolyshkin
Copy link
Collaborator

@eriksjolund is this still WIP? the description says so but I'm not sure it's up to date.

@eriksjolund
Copy link
Contributor Author

@eriksjolund is this still WIP? the description says so but I'm not sure it's up to date.

I removed the WIP from the description.

@eriksjolund eriksjolund force-pushed the document-error-handling branch from c789d63 to 1288a79 Compare February 6, 2026 07:43
@eriksjolund
Copy link
Contributor Author

Do you think we could also add an AGENTS.md file that links to this file?

Sure, I added a link in AGENTS.md

I also removed the last line in design-docs/error_handling.md
as it was a bit redundant.

@kolyshkin
Copy link
Collaborator

Test (check) failure is a flake; addressed by #1987

Functions that can return a failure to the caller, take the argument
`libcrun_error_t *err` and return an int or a pointer.

If the function succeeds, then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also describe a special case of returning a positive (> 0) int here?

Copy link
Contributor Author

@eriksjolund eriksjolund Feb 7, 2026

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@eriksjolund could you amend this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Closes: containers#1860

Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
@eriksjolund eriksjolund force-pushed the document-error-handling branch from 1288a79 to b0847b3 Compare March 16, 2026 19:07
Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kolyshkin kolyshkin merged commit d655ea6 into containers:main Mar 16, 2026
48 checks passed
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.

Document error handling convention

3 participants