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

RotatingFileHandler::_rotate doesn't handle rotation if file is opened in append mode #123

Closed
anluoma opened this issue Aug 4, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@anluoma
Copy link
Contributor

anluoma commented Aug 4, 2021

I am using RotatingFileHandler with append mode (a). This causes the rotation to stop working. Exceptions are:

failed to close previous log file during rotation, with error message errno: "17"
fwrite failed with error message errno: "9"

quill::Handler *file_handler = quill::rotating_file_handler(logDirFile, "a", maxFileSize, backupCount);

Problem seems to be that the rotate() function doesn't initialize the _currentIndex variable when booting up.
This causes rotation always trying to rename function to logfile.1.log which already exists and rename operation fails.

See the code from rotate function and the comment // then we will always rename the base filename to 1

// if we have more than 2 files we need to start renaming recursively
  for (uint32_t i = _current_index; i >= 1; --i)
  {
    filename_t const previous_file = detail::file_utilities::append_index_to_filename(_filename, i);
    filename_t const new_file = detail::file_utilities::append_index_to_filename(_filename, i + 1);

    quill::detail::rename(previous_file, new_file);
  }

  // then we will always rename the base filename to 1
  filename_t const previous_file = _filename;
  filename_t const new_file = detail::file_utilities::append_index_to_filename(_filename, 1);

  quill::detail::rename(previous_file, new_file);

If we open the file in 'w' mode the previous file gets overwritten and log writing is started ok.

Is this by design or can this be corrected?

@oPiZiL
Copy link

oPiZiL commented Dec 3, 2021

failed to rename previous log file during rotation, with error message errno: "2"

@odygrd
Copy link
Owner

odygrd commented Apr 15, 2022

I am not sure how to reproduce this.

I tried running the following :

int main()
{
  static char const* base_filename = "logfile.log";
  quill::start();
  quill::Handler* file_handler = quill::rotating_file_handler(base_filename, "w", 1024, 5);
  quill::Logger* logger_bar = quill::create_logger("rotating", file_handler);

  for (uint32_t i = 0; i < 20; ++i)
  {
    LOG_INFO(logger_bar, "Hello from {} {}", "rotating logger", i);
  }
}

Then i started my program again in append mode :

int main()
{
   static char const* base_filename = "logfile.log";
  quill::start();

  quill::Handler* file_handler = quill::rotating_file_handler(base_filename, "a", 1024, 5);
  quill::Logger* logger_bar = quill::create_logger("rotating", file_handler);

  for (uint32_t i = 20; i < 30; ++i)
  {
    LOG_INFO(logger_bar, "Hello from {} {}", "rotating logger", i);
  }
}

The above seems to be working and the log is appended. Can you maybe provide an example how to reproduce ?
What OS are you using ?

@anluoma
Copy link
Contributor Author

anluoma commented May 5, 2022

hi,

Sorry for delayed response. I was using Windows (10, 11 or server). I tried your example code it works the first time but second run is giving the error. So it need those file to be there and that make the index to be corrupted.

-antti

@anluoma
Copy link
Contributor Author

anluoma commented May 5, 2022

Was checking the releases and noticed #157 maybe its related.

@odygrd
Copy link
Owner

odygrd commented May 15, 2022

thanks for reporting. I have fixed that in the new upcoming version v2 ac62be2

However, v2 does have limited support for wide chars on windows at the moment. If you are using wide chars then you will have to stick to v1.7.x. There is no fix for this in v1.7.x as it would take a bit more work to do that cross platform for c++14 without std::filesystem.
Let me know if you can't use v2

@odygrd odygrd added the bug Something isn't working label May 18, 2022
@odygrd
Copy link
Owner

odygrd commented May 18, 2022

closing for now since there is a fix in v2. Please re-open if any issues

@odygrd odygrd closed this as completed May 18, 2022
@anluoma
Copy link
Contributor Author

anluoma commented May 19, 2022

Thanks for fixing! We do actually use wide and char but this is not currently issue as we changed to use Fileappend when we noticed this. I have see more closely this wide char thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants