-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix NaN issue in Execa by extracting .stdout
#629
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
docker/clone-and-preflight.ts
Outdated
const postgresUid = Number(await execa`id -u postgres`); | ||
const postgresUid = Number((await $`id -u postgres`).stdout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the execa
identifier isn't just the one imported directly from the 'execa'
package, but it's this:
const projectPath = 'project-to-check';
const execa = bindExeca({ cwd: projectPath });
so probably we need to stick with using the execa
identifier, or do a bigger change to switch everything to $
, with this binding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to execa
. cfc6c4e
While debugging this issue, we discovered that Execa await $`dep deploy --branch=${branch}`; // Works: `branch.stdout` is extracted correctly When assigning the result to a variable, Execa returns an object, not a string, which led to the NaN issue in our case const branch = await $`git branch --show-current`;
console.log(branch); // Logs: [object Object] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Closes #588
The Preflight Docker container failed when trying to set the
uid
for the PostgreSQL setup script. The issue was that Execa returns anobject
, andNumber()
was called on the object instead of just the command output, resulting inNaN
. This caused Execa to throw this error:'The "options.uid" property must be int32. Received type number (NaN)'
It wasn’t clear why the value was
NaN
. We assumed thatexeca
returned astring
, but it returns anobject
containing the command output under.stdout
. It failed becauseNumber()
was called on the entire object instead of extracting.stdout
first.To fix this error, we need to extract
.stdout
before converting to a number.const postgresUid = Number((await $`id -u postgres`).stdout); // Works: Extracts string output
ESLint didn’t catch this issue because
Number({})
is currently not flagged as an error. A proposal has been made to TypeScript ESLint to introduce a rule preventing passingobjects
toNumber()
.