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

libcnb-test: Improve error messages for address_for_port #636

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented 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.

Example output with libcnb-test v0.13.0:

thread 'basic' panicked at 'called `Option::unwrap()` on a `None` value', examples/ruby-sample/tests/integration_test.rs:38:63

With libcnb-test from main (that includes #605 and #621):

thread 'basic' panicked at 'Error obtaining container port mapping:

docker command failed with exit code 1!

## stderr:

Error: No public port '12346' published for libcnbtest_tydfxdispzng

## stdout:


', libcnb-test/src/container_context.rs:117:17

After this PR:

thread 'basic' panicked at 'Error obtaining container port mapping:
Error: No public port '12346' published for libcnbtest_giidbduyppom

This normally means that the container crashed. Container logs:

## stderr:

app.rb:1:in `<main>': Example server startup crash error message... (RuntimeError)

## stdout:


', libcnb-test/src/container_context.rs:130:17

Fixes #482.
GUS-W-13966399.

@edmorley edmorley added enhancement New feature or request libcnb-test labels Aug 17, 2023
@edmorley edmorley self-assigned this Aug 17, 2023
@edmorley edmorley marked this pull request as ready for review August 17, 2023 13:53
@edmorley edmorley requested a review from a team as a code owner August 17, 2023 13:53
@edmorley edmorley linked an issue Aug 17, 2023 that may be closed by this pull request
Base automatically changed from edmorley/log-output-display to main August 17, 2023 14:54
edmorley added a commit that referenced this pull request 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.
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.
@edmorley edmorley force-pushed the edmorley/address-for-port-container-logs branch from 84383bc to 22a8a34 Compare August 17, 2023 14:55
@edmorley edmorley enabled auto-merge (squash) August 17, 2023 14:58
@edmorley edmorley merged commit e99e3aa into main Aug 17, 2023
4 checks passed
@edmorley edmorley deleted the edmorley/address-for-port-container-logs branch August 17, 2023 14:59
colincasey added a commit that referenced this pull request Aug 18, 2023
* main:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
colincasey added a commit that referenced this pull request Aug 18, 2023
…cator

* main:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
#	libcnb-cargo/src/package/error.rs
#	libcnb-cargo/tests/test.rs
#	libcnb-package/src/lib.rs
colincasey added a commit that referenced this pull request Aug 18, 2023
… libcnb-package/assembling_buildpack_directories

* libcnb-package/buildpack_output_directory_locator:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
#	libcnb-package/src/output.rs
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
Development

Successfully merging this pull request may close these issues.

Confusing UX when using address_for_port on a container that's crashed
2 participants