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

Confusing UX when using address_for_port on a container that's crashed #482

Closed
edmorley opened this issue Jul 21, 2022 · 0 comments · Fixed by #636 or #605
Closed

Confusing UX when using address_for_port on a container that's crashed #482

edmorley opened this issue Jul 21, 2022 · 0 comments · Fixed by #636 or #605
Assignees
Labels
enhancement New feature or request libcnb-test

Comments

@edmorley
Copy link
Member

edmorley commented Jul 21, 2022

Currently, if a container started using TestContext::start_container crashes, when ContainerContext::address_for_port is used, it returns None - which means the typical .unwrap() call on the return value fails with a "Unwrapping None value" type error, which isn't very descriptive.

The fix for this might end up being related to #446 (ie: for long running containers, at least check they are still running before handing over control to the context), or else we could make the return type of ContainerContext be a Result where the error variant has a more descriptive message ("is this the correct port, or could the server have crashed?" etc).

As a workaround, I added assert_empty!(container.logs_now().stderr) type usages prior to the address_for_port call, to ensure any crash error messages cause an earlier (and easier to debug) failure than the address_for_port None unwrap.

GUS-W-13966399.

@edmorley edmorley self-assigned this Aug 17, 2023
@edmorley edmorley added the enhancement New feature or request label Aug 17, 2023
edmorley added a commit that referenced this issue Aug 17, 2023
Since:
- There was already duplication of formatting stderr/stdout output,
  which was otherwise going to get worse after #636.
- It may also be useful for end users when debugging, to save them
  having to manually print both stderr and stdout manually.

Prep for #482.
GUS-W-13966399.
edmorley added a commit that referenced this issue Aug 17, 2023
Improves the debugging UX for several `ContainerContext::address_for_port`
failure modes.

In particular, if containers crash after startup then the container logs
are now included in the panic error message, rather than only the
potentially confusing "No public port X published" message from Docker.

Fixes #482.
GUS-W-13966399.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request libcnb-test
Projects
None yet
1 participant