Skip to content
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

Throw errors instead of os.Exit of invalid fonts #833

Closed
vedantmamgain opened this issue Mar 18, 2024 · 13 comments
Closed

Throw errors instead of os.Exit of invalid fonts #833

vedantmamgain opened this issue Mar 18, 2024 · 13 comments

Comments

@vedantmamgain
Copy link

I faced an os.Exit when pdfcpu tried to perform pdf operation on a pdf with invalid font with the following log.
pdfcpu: user font not loaded: Arial.

I looked in the code an found the following line where we are doing an os.Exit instead of returning an error.

@hhrutter
Copy link
Collaborator

This is intentional!
Show me your stack trace.

@vedantmamgain
Copy link
Author

vedantmamgain commented Mar 18, 2024

@hhrutter I got pdfcpu: user font not loaded: Arial when trying to fill a pdf.

Why is this behaviour of throwing an os.Exit intentional, I'm using the PDFCPU API instead of CLI hence wouldn't we want to throw errors instead.

@hhrutter
Copy link
Collaborator

Show me your stack trace.
You should have gotten an error way before that!

@hhrutter
Copy link
Collaborator

hhrutter commented Mar 19, 2024

Bottomline: You have to ensure you have all user fonts installed that you will need for your operation.

I need more info about what you are doing in oder to analyze this.

@fancycode
Copy link
Contributor

Even if the API is used in an incompatible way, using os.Exit in library code is considered bad practice. Better would be to use panic(fmt.Sprintf("pdfcpu: user font not loaded: %s", fontName)) here so it could be catched by the application while still aborting a commandline tool.

@hhrutter
Copy link
Collaborator

This is a judgement call and pdfcpu is still Alpha.
It's more an indication for that you should never ever get into this situation.
Instead of digressing I'd still like to get to the root of this and in order to do so I need more info.

@vedantmamgain
Copy link
Author

@hhrutter There wasn't any other error before the os.Exit.

@hhrutter
Copy link
Collaborator

hhrutter commented Apr 3, 2024

I need to see what you are doing in order to follow up.

@vedantmamgain
Copy link
Author

vedantmamgain commented Apr 4, 2024

@hhrutter I'm using the pdfcpu api's fill function on pdfs which are fetched from s3. I got the above os.Exit message when trying to perform fill action on a pdf. Following is my model configuration

	modelConfig := model.NewDefaultConfiguration()
	modelConfig.Reader15 = true
	modelConfig.WriteXRefStream = true
	modelConfig.NeedAppearances = true

Also before filling, I'm performing the optimise action on the pdfs as well.

@hhrutter
Copy link
Collaborator

hhrutter commented Apr 5, 2024

There should have been a check if the font in question is installed as one of your pdfcpu user fonts prior to os.exiting.
In order to track that down I need to see a full stacktrace please.

@vedantmamgain
Copy link
Author

@hhrutter Are you talking about checks in pdfcpu or checks that I should implement. I've verified the error log and post downloading the file from s3 the only log that I can see is pdfcpu: user font not loaded: Arial%

@hhrutter
Copy link
Collaborator

Please go get the latest commit and report back any issues.
Thanks!

@vedantmamgain
Copy link
Author

@hhrutter Verified that this is now fixed

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

No branches or pull requests

3 participants