Skip to content

chore(test-registry): Add more descriptive error code for common error #16790

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Jul 2, 2025

The error code 137 is a very common error when running E2E tests. Starting colima only with colima start often results in too little memory and it makes sense to add a more descriptive message here.

To make it work with Colima, it's better to start it with colima start --cpu 10 --memory 12 --network-address

@s1gr1d s1gr1d requested a review from mydea July 2, 2025 12:40
@s1gr1d s1gr1d changed the title chore(registry): Add more descriptive error code for common error chore(test-registry): Add more descriptive error code for common error Jul 2, 2025
Comment on lines +88 to +96
const statusCode = publishImageContainerRunProcess.status;

if (statusCode !== 0) {
if (statusCode === 137) {
throw new Error(
`Publish Image Container failed with exit code ${statusCode}, possibly due to memory issues. Consider increasing the memory limit for the container.`,
);
}
throw new Error(`Publish Image Container failed with exit code ${statusCode}`);
Copy link

@bricefriha bricefriha Jul 2, 2025

Choose a reason for hiding this comment

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

I like the description, it's quite nice! 🙏🏼

For the sake of give my two cents: since throw won't will prevent the rest of the code to run, perhaps we may as well avoid the embedded if statements

Suggested change
const statusCode = publishImageContainerRunProcess.status;
if (statusCode !== 0) {
if (statusCode === 137) {
throw new Error(
`Publish Image Container failed with exit code ${statusCode}, possibly due to memory issues. Consider increasing the memory limit for the container.`,
);
}
throw new Error(`Publish Image Container failed with exit code ${statusCode}`);
const statusCode = publishImageContainerRunProcess.status;
if (statusCode !== 0) {
throw new Error(`Publish Image Container failed with exit code ${statusCode}`);
}
if (statusCode === 137) {
throw new Error(
`Publish Image Container failed with exit code ${statusCode}, possibly due to memory issues. Consider increasing the memory limit for the container.`,
);

It's not so much for the performance benefits (since that's for testing), but I find it easier to read, as far as I'm concerned 😁
Let me know if there is something I'm missing

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.

2 participants