This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 707f87ac Improve diagnostic hints when using zip/gzip to compress log
files (#405)
707f87ac is described below
commit 707f87acc9006f99723d1e69aab0626fbd73b5a5
Author: Stephen Webb <[email protected]>
AuthorDate: Mon Aug 5 11:52:43 2024 +1000
Improve diagnostic hints when using zip/gzip to compress log files (#405)
* Only emit error code when no description is available
* Report the name of the file not created
* Silence clang warnings
---
src/main/cpp/aprserversocket.cpp | 2 +-
src/main/cpp/exception.cpp | 34 ++++++++----
src/main/cpp/fileinputstream.cpp | 4 +-
src/main/cpp/fileoutputstream.cpp | 4 +-
src/main/cpp/gzcompressaction.cpp | 15 ++----
src/main/cpp/inputstreamreader.cpp | 2 +-
src/main/cpp/loggingevent.cpp | 3 +-
src/main/cpp/multiprocessrollingfileappender.cpp | 61 ++++++++++-----------
src/main/cpp/onlyonceerrorhandler.cpp | 4 +-
src/main/cpp/rollingfileappender.cpp | 68 +++++++++++-------------
src/main/cpp/zipcompressaction.cpp | 9 ++--
src/main/include/log4cxx/helpers/exception.h | 2 +
12 files changed, 108 insertions(+), 100 deletions(-)
diff --git a/src/main/cpp/aprserversocket.cpp b/src/main/cpp/aprserversocket.cpp
index 580f757f..e9d49c0c 100644
--- a/src/main/cpp/aprserversocket.cpp
+++ b/src/main/cpp/aprserversocket.cpp
@@ -111,7 +111,7 @@ SocketPtr APRServerSocket::accept()
}
if (s == 0)
{
- throw IOException(LOG4CXX_STR("socket is 0"));
+ throw NullPointerException(LOG4CXX_STR("socket"));
}
apr_pollfd_t poll;
diff --git a/src/main/cpp/exception.cpp b/src/main/cpp/exception.cpp
index e70ad6a4..e45c3294 100644
--- a/src/main/cpp/exception.cpp
+++ b/src/main/cpp/exception.cpp
@@ -153,6 +153,11 @@ IOException::IOException(log4cxx_status_t stat)
{
}
+IOException::IOException(const LogString& type, log4cxx_status_t stat)
+ : Exception(formatMessage(type, stat))
+{
+}
+
IOException::IOException(const LogString& msg1)
: Exception(msg1)
@@ -172,16 +177,27 @@ IOException& IOException::operator=(const IOException&
src)
LogString IOException::formatMessage(log4cxx_status_t stat)
{
- char err_buff[1024];
- LogString s(LOG4CXX_STR("IO Exception : status code = "));
- Pool p;
- StringHelper::toString(stat, p, s);
- s.append(LOG4CXX_STR("("));
+ return formatMessage(LOG4CXX_STR("IO Exception"), stat);
+}
+
+LogString IOException::formatMessage(const LogString& type, log4cxx_status_t
stat)
+{
+ LogString s = type;
+ char err_buff[1024] = {0};
apr_strerror(stat, err_buff, sizeof(err_buff));
- std::string sMsg = err_buff;
- LOG4CXX_DECODE_CHAR(lsMsg, sMsg);
- s.append(lsMsg);
- s.append(LOG4CXX_STR(")"));
+ if (0 == err_buff[0] || 0 == strncmp(err_buff, "APR does not
understand", 23))
+ {
+ s.append(LOG4CXX_STR(": error code "));
+ Pool p;
+ StringHelper::toString(stat, p, s);
+ }
+ else
+ {
+ s.append(LOG4CXX_STR(" - "));
+ std::string sMsg = err_buff;
+ LOG4CXX_DECODE_CHAR(lsMsg, sMsg);
+ s.append(lsMsg);
+ }
return s;
}
diff --git a/src/main/cpp/fileinputstream.cpp b/src/main/cpp/fileinputstream.cpp
index 248006e4..f9755669 100644
--- a/src/main/cpp/fileinputstream.cpp
+++ b/src/main/cpp/fileinputstream.cpp
@@ -61,7 +61,7 @@ void FileInputStream::open(const LogString& filename)
if (stat != APR_SUCCESS)
{
- throw IOException(stat);
+ throw IOException(filename, stat);
}
}
@@ -75,7 +75,7 @@ FileInputStream::FileInputStream(const File& aFile) :
if (stat != APR_SUCCESS)
{
- throw IOException(stat);
+ throw IOException(aFile.getName(), stat);
}
}
diff --git a/src/main/cpp/fileoutputstream.cpp
b/src/main/cpp/fileoutputstream.cpp
index 5becde65..b93f71ec 100644
--- a/src/main/cpp/fileoutputstream.cpp
+++ b/src/main/cpp/fileoutputstream.cpp
@@ -73,7 +73,7 @@ apr_file_t* FileOutputStream::open(const LogString& filename,
if (stat != APR_SUCCESS)
{
- throw IOException(stat);
+ throw IOException(filename, stat);
}
return fileptr;
@@ -110,7 +110,7 @@ void FileOutputStream::write(ByteBuffer& buf, Pool& /* p */
)
{
if (m_priv->fileptr == NULL)
{
- throw IOException(-1);
+ throw NullPointerException(LOG4CXX_STR("FileOutputStream"));
}
size_t nbytes = buf.remaining();
diff --git a/src/main/cpp/gzcompressaction.cpp
b/src/main/cpp/gzcompressaction.cpp
index bc07e13d..8b30690f 100644
--- a/src/main/cpp/gzcompressaction.cpp
+++ b/src/main/cpp/gzcompressaction.cpp
@@ -91,7 +91,7 @@ bool GZCompressAction::execute(LOG4CXX_NS::helpers::Pool& p)
const
if (stat != APR_SUCCESS)
{
- throw IOException(stat);
+ throw IOException(priv->destination.getName(), stat);
}
stat = apr_procattr_child_out_set(attr, child_out, NULL);
@@ -130,11 +130,11 @@ bool GZCompressAction::execute(LOG4CXX_NS::helpers::Pool&
p) const
apr_proc_t pid;
stat = apr_proc_create(&pid, "gzip", args, NULL, attr, aprpool);
- if (stat != APR_SUCCESS && priv->throwIOExceptionOnForkFailure)
- {
- throw IOException(stat);
- }else if(stat != APR_SUCCESS &&
!priv->throwIOExceptionOnForkFailure)
+ if (stat != APR_SUCCESS)
{
+ LogLog::warn(LOG4CXX_STR("Failed to fork gzip during
log rotation; leaving log file uncompressed"));
+ if (priv->throwIOExceptionOnForkFailure)
+ throw IOException(LOG4CXX_STR("gzip"), stat);
/* If we fail here (to create the gzip child process),
* skip the compression and consider the rotation to be
* otherwise successful. The caller has already rotated
@@ -143,12 +143,7 @@ bool GZCompressAction::execute(LOG4CXX_NS::helpers::Pool&
p) const
* same path with `.gz` appended). Remove the empty
* destination file and leave source as-is.
*/
- LogLog::warn(LOG4CXX_STR("Failed to fork gzip during
log rotation; leaving log file uncompressed"));
stat = apr_file_close(child_out);
- if (stat != APR_SUCCESS)
- {
- LogLog::warn(LOG4CXX_STR("Failed to close
abandoned .gz file; ignoring"));
- }
return true;
}
diff --git a/src/main/cpp/inputstreamreader.cpp
b/src/main/cpp/inputstreamreader.cpp
index 9027fbfc..e300202a 100644
--- a/src/main/cpp/inputstreamreader.cpp
+++ b/src/main/cpp/inputstreamreader.cpp
@@ -86,7 +86,7 @@ LogString InputStreamReader::read(Pool& p)
if (stat != 0)
{
- throw IOException(stat);
+ throw IOException(LOG4CXX_STR("decode"), stat);
}
if (buf.remaining() > 0)
diff --git a/src/main/cpp/loggingevent.cpp b/src/main/cpp/loggingevent.cpp
index e76ded9b..a13fc661 100644
--- a/src/main/cpp/loggingevent.cpp
+++ b/src/main/cpp/loggingevent.cpp
@@ -206,7 +206,8 @@ bool LoggingEvent::getNDC(LogString& dest) const
// Otherwise use the NDC that is associated with the thread.
if (m_priv->dc)
{
- if (result = m_priv->dc->ctx.has_value())
+ result = m_priv->dc->ctx.has_value();
+ if (result)
dest.append(NDC::getFullMessage(m_priv->dc->ctx.value()));
}
else
diff --git a/src/main/cpp/multiprocessrollingfileappender.cpp
b/src/main/cpp/multiprocessrollingfileappender.cpp
index b20b8589..9614bb2b 100644
--- a/src/main/cpp/multiprocessrollingfileappender.cpp
+++ b/src/main/cpp/multiprocessrollingfileappender.cpp
@@ -336,16 +336,18 @@ bool
MultiprocessRollingFileAppender::rolloverInternal(Pool& p)
}
catch
(std::exception& ex)
{
-
LogLog::warn(LOG4CXX_STR("Exception on rollover"));
-
LogString exmsg;
-
LOG4CXX_NS::helpers::Transcoder::decode(ex.what(), exmsg);
-
_priv->errorHandler->error(exmsg, ex, 0);
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
}
}
+ bool appendToExisting =
true;
if (success)
{
- if
(rollover1->getAppend())
+
appendToExisting = rollover1->getAppend();
+ if
(appendToExisting)
{
_priv->fileLength = File().setPath(rollover1->getActiveFileName()).length(p);
}
@@ -354,25 +356,24 @@ bool
MultiprocessRollingFileAppender::rolloverInternal(Pool& p)
_priv->fileLength = 0;
}
- //
- // async
action not yet implemented
- //
ActionPtr
asyncAction(rollover1->getAsynchronous());
if (asyncAction
!= NULL)
{
-
asyncAction->execute(p);
+ try
+ {
+
asyncAction->execute(p);
+ }
+ catch
(std::exception& ex)
+ {
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
+ }
}
-
- setFileInternal(
-
rollover1->getActiveFileName(), rollover1->getAppend(),
-
_priv->bufferedIO, _priv->bufferSize, p);
- }
- else
- {
- setFileInternal(
-
rollover1->getActiveFileName(), true, _priv->bufferedIO, _priv->bufferSize, p);
}
+
setFileInternal(rollover1->getActiveFileName(), appendToExisting,
_priv->bufferedIO, _priv->bufferSize, p);
}
else
{
@@ -397,10 +398,10 @@ bool
MultiprocessRollingFileAppender::rolloverInternal(Pool& p)
}
catch
(std::exception& ex)
{
-
LogLog::warn(LOG4CXX_STR("Exception during rollover"));
-
LogString exmsg;
-
LOG4CXX_NS::helpers::Transcoder::decode(ex.what(), exmsg);
-
_priv->errorHandler->error(exmsg, ex, 0);
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
}
}
@@ -435,10 +436,10 @@ bool
MultiprocessRollingFileAppender::rolloverInternal(Pool& p)
}
catch (std::exception& ex)
{
- LogLog::warn(LOG4CXX_STR("Exception
during rollover"));
- LogString exmsg;
-
LOG4CXX_NS::helpers::Transcoder::decode(ex.what(), exmsg);
- _priv->errorHandler->error(exmsg, ex,
0);
+ LogString msg(LOG4CXX_STR("Rollover of
["));
+ msg.append(getFile());
+ msg.append(LOG4CXX_STR("] failed"));
+ _priv->errorHandler->error(msg, ex, 0);
}
}
@@ -492,10 +493,10 @@ void MultiprocessRollingFileAppender::subAppend(const
LoggingEventPtr& event, Po
}
catch (std::exception& ex)
{
- LogLog::warn(LOG4CXX_STR("Exception during rollover
attempt."));
- LogString exmsg;
- LOG4CXX_NS::helpers::Transcoder::decode(ex.what(),
exmsg);
- _priv->errorHandler->error(exmsg);
+ LogString msg(LOG4CXX_STR("Rollover of ["));
+ msg.append(getFile());
+ msg.append(LOG4CXX_STR("] failed"));
+ _priv->errorHandler->error(msg, ex, 0);
}
}
diff --git a/src/main/cpp/onlyonceerrorhandler.cpp
b/src/main/cpp/onlyonceerrorhandler.cpp
index a124ede7..20c63729 100644
--- a/src/main/cpp/onlyonceerrorhandler.cpp
+++ b/src/main/cpp/onlyonceerrorhandler.cpp
@@ -28,12 +28,12 @@ IMPLEMENT_LOG4CXX_OBJECT(OnlyOnceErrorHandler)
struct OnlyOnceErrorHandler::OnlyOnceErrorHandlerPrivate{
OnlyOnceErrorHandlerPrivate() :
- WARN_PREFIX(LOG4CXX_STR("log4cxx warning: ")),
- ERROR_PREFIX(LOG4CXX_STR("log4cxx error: ")),
firstTime(true){}
+#if LOG4CXX_ABI_VERSION <= 15
LogString WARN_PREFIX;
LogString ERROR_PREFIX;
+#endif
mutable bool firstTime;
};
diff --git a/src/main/cpp/rollingfileappender.cpp
b/src/main/cpp/rollingfileappender.cpp
index 21f931c4..f96fbcba 100644
--- a/src/main/cpp/rollingfileappender.cpp
+++ b/src/main/cpp/rollingfileappender.cpp
@@ -336,17 +336,18 @@ bool RollingFileAppender::rolloverInternal(Pool& p)
}
catch
(std::exception& ex)
{
-
LOG4CXX_DECODE_CHAR(lsMsg, ex.what());
-
LogString errorMsg = LOG4CXX_STR("Exception on rollover: ");
-
errorMsg.append(lsMsg);
-
LogLog::error(errorMsg);
-
_priv->errorHandler->error(lsMsg, ex, 0);
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
}
}
+ bool appendToExisting =
true;
if (success)
{
- if
(rollover1->getAppend())
+
appendToExisting = rollover1->getAppend();
+ if
(appendToExisting)
{
_priv->fileLength = File().setPath(rollover1->getActiveFileName()).length(p);
}
@@ -355,25 +356,24 @@ bool RollingFileAppender::rolloverInternal(Pool& p)
_priv->fileLength = 0;
}
- //
- // async
action not yet implemented
- //
ActionPtr
asyncAction(rollover1->getAsynchronous());
if (asyncAction
!= NULL)
{
-
asyncAction->execute(p);
+ try
+ {
+
asyncAction->execute(p);
+ }
+ catch
(std::exception& ex)
+ {
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
+ }
}
-
- setFileInternal(
-
rollover1->getActiveFileName(), rollover1->getAppend(),
-
_priv->bufferedIO, _priv->bufferSize, p);
- }
- else
- {
- setFileInternal(
-
rollover1->getActiveFileName(), true, _priv->bufferedIO, _priv->bufferSize, p);
}
+
setFileInternal(rollover1->getActiveFileName(), appendToExisting,
_priv->bufferedIO, _priv->bufferSize, p);
}
else
{
@@ -398,11 +398,10 @@ bool RollingFileAppender::rolloverInternal(Pool& p)
}
catch
(std::exception& ex)
{
-
LOG4CXX_DECODE_CHAR(lsMsg, ex.what());
-
LogString errorMsg = LOG4CXX_STR("Exception during rollover: ");
-
errorMsg.append(lsMsg);
-
LogLog::warn(errorMsg);
-
_priv->errorHandler->error(lsMsg, ex, 0);
+
LogString msg(LOG4CXX_STR("Rollover of ["));
+
msg.append(getFile());
+
msg.append(LOG4CXX_STR("] failed"));
+
_priv->errorHandler->error(msg, ex, 0);
}
}
@@ -417,9 +416,6 @@ bool RollingFileAppender::rolloverInternal(Pool& p)
_priv->fileLength = 0;
}
- //
- // async
action not yet implemented
- //
ActionPtr
asyncAction(rollover1->getAsynchronous());
if (asyncAction
!= NULL)
@@ -435,11 +431,10 @@ bool RollingFileAppender::rolloverInternal(Pool& p)
}
catch (std::exception& ex)
{
- LOG4CXX_DECODE_CHAR(lsMsg, ex.what());
- LogString errorMsg =
LOG4CXX_STR("Exception during rollover: ");
- errorMsg.append(lsMsg);
- LogLog::warn(errorMsg);
- _priv->errorHandler->error(lsMsg, ex,
0);
+ LogString msg(LOG4CXX_STR("Rollover of
["));
+ msg.append(getFile());
+ msg.append(LOG4CXX_STR("] failed"));
+ _priv->errorHandler->error(msg, ex, 0);
}
}
}
@@ -470,11 +465,10 @@ void RollingFileAppender::subAppend(const
LoggingEventPtr& event, Pool& p)
}
catch (std::exception& ex)
{
- LOG4CXX_DECODE_CHAR(lsMsg, ex.what());
- LogString errorMsg = LOG4CXX_STR("Exception during
rollover attempt: ");
- errorMsg.append(lsMsg);
- LogLog::warn(errorMsg);
- _priv->errorHandler->error(lsMsg);
+ LogString msg(LOG4CXX_STR("Rollover of ["));
+ msg.append(getFile());
+ msg.append(LOG4CXX_STR("] failed"));
+ _priv->errorHandler->error(msg, ex, 0);
}
}
diff --git a/src/main/cpp/zipcompressaction.cpp
b/src/main/cpp/zipcompressaction.cpp
index 2ba9e729..febb2004 100644
--- a/src/main/cpp/zipcompressaction.cpp
+++ b/src/main/cpp/zipcompressaction.cpp
@@ -116,11 +116,11 @@ bool
ZipCompressAction::execute(LOG4CXX_NS::helpers::Pool& p) const
apr_proc_t pid;
stat = apr_proc_create(&pid, "zip", args, NULL, attr, aprpool);
- if (stat != APR_SUCCESS && priv->throwIOExceptionOnForkFailure)
- {
- throw IOException(stat);
- }else if(stat != APR_SUCCESS && !priv->throwIOExceptionOnForkFailure)
+ if (stat != APR_SUCCESS)
{
+ LogLog::warn(LOG4CXX_STR("Failed to fork zip during log
rotation; leaving log file uncompressed"));
+ if (priv->throwIOExceptionOnForkFailure)
+ throw IOException(LOG4CXX_STR("zip"), stat);
/* If we fail here (to create the zip child process),
* skip the compression and consider the rotation to be
* otherwise successful. The caller has already rotated
@@ -129,7 +129,6 @@ bool ZipCompressAction::execute(LOG4CXX_NS::helpers::Pool&
p) const
* same path with `.zip` appended). Remove the empty
* destination file and leave source as-is.
*/
- LogLog::warn(LOG4CXX_STR("Failed to fork zip during log
rotation; leaving log file uncompressed"));
return true;
}
diff --git a/src/main/include/log4cxx/helpers/exception.h
b/src/main/include/log4cxx/helpers/exception.h
index 4bf5b84a..08f48d16 100644
--- a/src/main/include/log4cxx/helpers/exception.h
+++ b/src/main/include/log4cxx/helpers/exception.h
@@ -92,10 +92,12 @@ class LOG4CXX_EXPORT IOException : public Exception
IOException();
IOException(log4cxx_status_t stat);
IOException(const LogString& msg);
+ IOException(const LogString& type, log4cxx_status_t stat);
IOException(const IOException& src);
IOException& operator=(const IOException&);
private:
static LogString formatMessage(log4cxx_status_t stat);
+ static LogString formatMessage(const LogString& type,
log4cxx_status_t stat);
};
class LOG4CXX_EXPORT MissingResourceException : public Exception
