fix: 🐛 add error handling for docker service startup failures#1880
fix: 🐛 add error handling for docker service startup failures#1880chronark merged 10 commits intounkeyed:mainfrom
Conversation
|
|
@DeepaPrasanna is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
This is better than what we currently have, but it would be nice if we can return the original error, so the user actually has something to work with |
|
The error was only a code with value 1, so that's why I thought it will be unclear to return that value. |
|
docker definitely returns more descriptive errors than that |
|
Hmm, maybe I missed. I will have a look at it again. |
|
Is this okay? I am now thinking of throwing the original error 
|
|
that doesn't help a developer. we need to return the actual error from the command that ran, not the error from the |
|
did u meant the |
|
yes |
|
In that case, It was empty. I will attach a screenshot. That why I went with Edit: It seems docker prints errors to |
|
shall i return |
|
yeah that makes sense I think |
|
Is this okay?
|
|
👍 |
|
I pushed the required changes. |
tools/local/src/docker.ts
Outdated
| const p = exec(cmd, opts, (error, stdout) => { | ||
| if (error) { | ||
| // biome-ignore lint/suspicious/noConsoleLog: logging seems the best option here | ||
| console.log(stdout); |
There was a problem hiding this comment.
it's an error, why don't we use console.error?
There was a problem hiding this comment.
Yes, that should have been used
2 participants
What does this PR do?
Fixes #1877
Updated the code to handle errors when starting docker services. If the service startup fails, an error is thrown with a descriptive message indicating which service encountered the issue.
Type of change
How should this be tested?
pnpm localwithout running docker.Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated