Github Copilot generated comment
Root cause analysis: Why 0-byte files get created when an article fails
The 0-byte file issue arises due to this code flow in daemon/nntp/ArticleWriter.cpp, specifically in the ArticleWriter::CompleteFileParts() function. Here is a breakdown of the behavior and where the bug comes from:
Key code locations
1. File is always opened even if nothing will be written:
CompleteFileParts() file open (line 409)
if (!g_Options->GetRawArticle() && !directWrite)
{
FileSystem::DeleteFile(tmpdestfile.c_str());
if (!outfile.Open(tmpdestfile.c_str(), DiskFile::omWrite))
{
m_fileInfo->GetNzbInfo()->PrintMessage(Message::mkError,
"Could not create file %s: %s",
tmpdestfile.c_str(),
*FileSystem::GetLastErrorMessage()
);
return;
}
}
This happens before any articles are checked for success or written.
2. All failed articles are skipped:
CompleteFileParts() article loop (line 468)
for (ArticleInfo* pa : m_fileInfo->GetArticles())
{
if (pa->GetStatus() != ArticleInfo::aiFinished)
{
continue;
}
// ... write ...
}
If no articles succeed (e.g. DMCA/NFO failure), the output file from above is left empty.
3. File is still renamed to the final filename:
CompleteFileParts() file close and rename (line 539)
if (outfile.Active())
{
outfile.Close();
if (!directWrite && !FileSystem::MoveFile(tmpdestfile.c_str(), ofn.c_str()))
{
m_fileInfo->GetNzbInfo()->PrintMessage(Message::mkError,
"Could not move file %s to %s: %s",
tmpdestfile.c_str(),
ofn.c_str(),
*FileSystem::GetLastErrorMessage()
);
}
}
Thus, the 0-byte file gets renamed to its final destination.
4. DirectWrite mode is also affected:
When DirectWrite is enabled, CreateOutputFile() creates the file ahead of download attempts. If the download then fails, the empty file is left and renamed similarly.
Why this happens
There is NO check in the function for "did any successful article data actually get written?" before renaming the file. So a 0-byte file is always left behind if all downloads fail.
What would fix it?
- After looping through articles, check if any data was written or if any article was in a success state before performing the rename/move.
- If none were, delete the file and do not move/rename it to the final location.
- This check is needed for both normal and direct write logic.
References:
Github Copilot generated comment
Root cause analysis: Why 0-byte files get created when an article fails
The 0-byte file issue arises due to this code flow in
daemon/nntp/ArticleWriter.cpp, specifically in theArticleWriter::CompleteFileParts()function. Here is a breakdown of the behavior and where the bug comes from:Key code locations
1. File is always opened even if nothing will be written:
CompleteFileParts()file open (line 409)This happens before any articles are checked for success or written.
2. All failed articles are skipped:
CompleteFileParts()article loop (line 468)If no articles succeed (e.g. DMCA/NFO failure), the output file from above is left empty.
3. File is still renamed to the final filename:
CompleteFileParts()file close and rename (line 539)Thus, the 0-byte file gets renamed to its final destination.
4. DirectWrite mode is also affected:
When
DirectWriteis enabled,CreateOutputFile()creates the file ahead of download attempts. If the download then fails, the empty file is left and renamed similarly.Why this happens
There is NO check in the function for "did any successful article data actually get written?" before renaming the file. So a 0-byte file is always left behind if all downloads fail.
What would fix it?
References:
CompleteFileParts()full sourceStart()file open logicCreateOutputFile()