erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

Call to error_logger:format with too few arguments #359

Closed etnt closed 5 years ago

etnt commented 5 years ago

https://github.com/klacke/yaws/blob/6ad15183e3783566b3e3a217aaffe9b59cc8c6ab/src/yaws_dynopts.erl#L232

vinoski commented 5 years ago

Thanks, will fix shortly. How did you find it? Is there a scenario I can run to try to cause a failure from this?

etnt commented 5 years ago

Well, I'm in the progress of upgrading our use of Yaws to the latest and ran into this because of a test case of ours that checks that the error logs are empty. The reason for ending up in this part of the code (i.e writing the file went ok, but compilation failed), was caused by the fact that in our release packages those Yaws .hrl files, that the generated code is including, are not part of our release package.

Regarding the 'generate' trick of yaws_dynopts.erl, I have some suggestions:

  1. Make it optional to generate this file.
  2. If not (1), remove the .hrl dependency by using calls to yaws_api.erl
  3. If not (1), make it possible to specify a directory to hold this file (currently /tmp is hardcoded)

I wonder how much is gained in performance by having this file generated like this? (we are considering to simply remove this generate step in our release builds)

vinoski commented 5 years ago

Regarding your suggestions:

  1. It should be relatively easy to avoid generating the file by adding a new global config setting. I'll look into it.

My answer to number 1 means questions 2 and 3 are moot, but I also wanted to note for 2 that even if we modified the module to call yaws_api, we'd still need the .hrl dependency to bring in the gconf record definition.