rxdi / firelink

Firebase. gcloud and monorepos are not combining very well until they met @rxdi/firelink
MIT License
68 stars 9 forks source link

Firelink Swallows All Errors #27

Closed hspak closed 2 years ago

hspak commented 2 years ago

Hello!

We are running firelink in CI to wrap firebase -- and it's eating all firebase errors I believe due to the try/catch covering the entire runtime of the program. It's causing errors to silently fail in CI.

firelink should propagate errors through.

Stradivario commented 2 years ago

Ouch! That doesn't sound right at all!

Kind of strange since we are piping the errors directly from the stream

https://github.com/rxdi/firelink/blob/master/src/helpers/worker.ts#L16

 child.stderr.pipe(process.stderr);
 child.stdout.pipe(process.stdout);

So i think errors are present inside the log but the actual exit status is incorrectly handled.

I will check that now

https://github.com/rxdi/firelink/blob/master/src/main.ts#L19

Lets remove this try/catch ;)

Stradivario commented 2 years ago

PR Introduced https://github.com/rxdi/firelink/pull/28 !

Merged in master we are expecting in about 10 minutes to have a new version of @rxdi/firelink@0.7.58

Please tell me how it is with the new version so we can move further :)

Cheers!

hspak commented 2 years ago

Thank you for looking into this!

Unfortunately, it looks like the problem persists in the new version (0.7.58).

I tested by adding "predeploy": "exit 1" to firebase.json, but the firebase deploy in CI still exited with 0.

i  deploying functions
Running command: exit 1

Error: functions predeploy error: Command terminated with non-zero exit code1
true
Stradivario commented 2 years ago

Kind of strange i will look into this!

Thanks for testing things out !

I will try to reproduce it in a CI/CD

Cheers!

hspak commented 2 years ago

I think the issue might be here: https://github.com/rxdi/firelink/blob/master/src/helpers/exit-handler.ts#L12

The explicit call to process.exit() is always going to return exit code 0.

hspak commented 2 years ago

Can confirm https://github.com/rxdi/firelink/pull/30 fixes it!