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

Exception noise from OAuth and REST (API2 ) #4501

Open
kiatng opened this issue Jan 21, 2025 · 4 comments
Open

Exception noise from OAuth and REST (API2 ) #4501

kiatng opened this issue Jan 21, 2025 · 4 comments

Comments

@kiatng
Copy link
Contributor

kiatng commented Jan 21, 2025

I have a project with OAuth REST which is interfacing with a mobile app. The exception.log is filling up with millions of lines due to unnecessary logs:

Mage_Api2_Exception: oauth_problem=token_rejected in .../app/code/core/Mage/Api2/Model/Auth/Adapter/Oauth.php:49
Mage_Api2_Exception: Missing required params in .../app/code/core/Mage/Api2/Model/Resource.php:629
Mage_Oauth_Exception in .../app/Mage.php:648, .../app/code/core/Mage/Oauth/Model/Server.php(421)

Each exception has 11 lines or more.

Preconditions (*)

Any version.

Steps to reproduce (*)

May be by unit tests?

Expected result (*)

  1. Do not Mage::logException($e) for Mage_Api2_Exception and Mage_Oauth_Exception?
  2. Only log one line per exception?

Actual result (*)

Noise in exception.log making it hard to detect the signal, problems that require attention.

@sreichel
Copy link
Contributor

Mage_Api2_Exception: Missing required params in .../app/code/core/Mage/Api2/Model/Resource.php:629

Please post that lines. Looks like modified code. (?)

@kiatng
Copy link
Contributor Author

kiatng commented Jan 22, 2025

Please post that lines. Looks like modified code. (?)

The project has lots of custom API2. When the client requests certain info but fail some validations, an exception is thrown, an example of bad request:

$this->_critical('Missing nationality', Mage_Api2_Model_Server::HTTP_BAD_REQUEST);

protected function _critical($message, $code = null)
{
if ($code === null) {
$errors = $this->_getCriticalErrors();
if (!isset($errors[$message])) {
throw new Exception(
sprintf('Invalid error "%s" or error code missed.', $message),
Mage_Api2_Model_Server::HTTP_INTERNAL_ERROR,
);
}
$code = $errors[$message];
}
throw new Mage_Api2_Exception($message, $code);
}

Line 629 is throw new Mage_Api2_Exception($message, $code); in my project.

@sreichel
Copy link
Contributor

Line 629 is throw new Mage_Api2_Exception($message, $code); in my project.

In core thats line 625 ...

Isnt it correct to log "$this->_critical('Missing nationality'"?

@kiatng
Copy link
Contributor Author

kiatng commented Feb 13, 2025

Sure, it is OK to log, but it's unnecessary to include the trace in the log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants