search

strongdm / comply

Compliance automation framework, focused on SOC2
https://comply.strongdm.com
Apache License 2.0
1.32k stars 248 forks source link

Fix hang on pandoc error. #104

Closed statik closed 3 years ago

statik commented 3 years ago

This fixes ticket #100.

When pandoc has an error, this code does a blocking send to the error channel:

https://github.com/strongdm/comply/blob/7b75522ee02ba62ecac26f8a758e55e9d22077c6/internal/render/pandoc.go#L21-L24

However, this error channel is never read from:

https://github.com/strongdm/comply/blob/7b75522ee02ba62ecac26f8a758e55e9d22077c6/internal/render/pdf.go#L13

This PR changes things to use the channel passed in from the parent, and with this change comply exits at the first pandoc error instead of hanging. There are definitely other approaches that could be used to fix the hang, but I think this one seems safe and reasonable.

Also related, until #98 is merged anyone setting up a new comply repo is likely to hit pandoc errors (and therefore run into #100).

statik commented 3 years ago

also mucho thanks to @robert-wallis for the goroutine debugging help!

statik commented 3 years ago

@camposer I believe this PR is important to merge, if there is anything I can do to assist, including describing the debugging and testing that I have done, I'm happy to try and make the review easier.

camposer commented 3 years ago

Thank you very much @statik! I'm actually planning to take a look at it tomorrow morning, I'll reach out without hesitation if needed :-)

statik commented 3 years ago

this was fixed a different way in #108