[16.0][ADD] fastapi_log by SirPyTech · Pull Request #554 · OCA/rest-framework

SirPyTech · 2025-08-04T10:10:51Z

In the past months we have been proposing many improvements to #501 with akretion#8.

From more than a month ago, the branch is outdated (akretion#8 (comment)) and it is causing conflicts in akretion#8.

Here we are proposing all those improvements to OCA directly.

SirPyTech · 2025-08-04T10:14:00Z

@simahawk here I addressed your latest comments akretion#8 (comment) and akretion#8 (comment), please have a look

simahawk · 2025-08-12T07:58:44Z

+        try:
+            headers_dict = {key: value for key, value in headers.items()}
+            return self._sanitize_headers_dict(headers_dict)
+        except AttributeError:


when do you expect to have this kind of error?

I'm suspicious about the headers argument, if it does not have the items method then an AttributeError would be raised.
But you're right, let's be optimistic and remove the exception handling 😄

simahawk · 2025-08-12T08:03:27Z

+    def _prepare_log_response(self, response):
+        return {
+            "response_status_code": response.status_code,
+            "response_headers": self._headers_to_dict(response.headers),


One of the things done in rest_log was to inject a reference to the log entry in the response.
There we assumed the response was always JSON data and added a key.
Here we could inject a response header (eg: API_LOG_ENTRY_URL)

I have added the identifier (api.log.id) of the log record, that is enough to find the log record to whoever has access to them.

Please keep in mind that I agree with the author of #501 that wrote:

This is not a migration of rest_log, this module aims to be simpler, at least in the first iterations.

A more elaborated feature (like building the full URL of the log record) can be added later, after the first simple module is merged.

simahawk · 2025-08-12T08:06:07Z

+    def _compute_log_ids(self):
+        for collection in self:
+            collection.log_ids = self.env["api.log"].search(
+                [("collection_ref", "=", "%s,%s" % (collection._name, collection.id))]


see my comment above on how to search for logs

simahawk · 2025-08-12T08:07:23Z

+from odoo import api, models
+
+
+class FastapiLog(models.Model):


Suggested change

class FastapiLog(models.Model):

class ApiLog(models.Model):

Nice catch! Thanks

SirPyTech · 2025-08-21T10:27:22Z

The error in the tests (https://github.com/OCA/rest-framework/actions/runs/17124124402/job/48571725570?pr=554#step:8:120) does not seem to be related to these changes

Stack

2025-08-21 10:24:04,335 340 INFO odoo odoo.modules.loading: Loading module graphql_base (12/53) 
2025-08-21 10:24:04,475 340 CRITICAL odoo odoo.modules.module: Couldn't load module graphql_base 
2025-08-21 10:24:04,475 340 CRITICAL odoo odoo.modules.module: cannot import name 'HttpQueryError' from 'graphql_server' (/opt/odoo-venv/lib/python3.10/site-packages/graphql_server/__init__.py) 
2025-08-21 10:24:04,477 340 WARNING odoo odoo.modules.loading: Transient module states were reset 
2025-08-21 10:24:04,477 340 ERROR odoo odoo.modules.registry: Failed to load registry 
Traceback (most recent call last):
  File "/opt/odoo/odoo/modules/registry.py", line 87, in new
    odoo.modules.load_modules(registry, force_demo, status, update_module)
  File "/opt/odoo/odoo/modules/loading.py", line 493, in load_modules
    processed_modules += load_marked_modules(cr, graph,
  File "/opt/odoo/odoo/modules/loading.py", line 374, in load_marked_modules
    loaded, processed = load_module_graph(
  File "/opt/odoo/odoo/modules/loading.py", line 190, in load_module_graph
    load_openerp_module(package.name)
  File "/opt/odoo/odoo/modules/module.py", line 471, in load_openerp_module
    __import__('odoo.addons.' + module_name)
  File "/__w/rest-framework/rest-framework/graphql_base/__init__.py", line 4, in 
    from .controllers import GraphQLControllerMixin
  File "/__w/rest-framework/rest-framework/graphql_base/controllers/__init__.py", line 1, in 
    from .main import GraphQLControllerMixin
  File "/__w/rest-framework/rest-framework/graphql_base/controllers/main.py", line 6, in 
    from graphql_server import (
ImportError: cannot import name 'HttpQueryError' from 'graphql_server' (/opt/odoo-venv/lib/python3.10/site-packages/graphql_server/__init__.py)
2025-08-21 10:24:04,478 340 CRITICAL odoo odoo.service.server: Failed to initialize database `odoo`. 
Traceback (most recent call last):
  File "/opt/odoo/odoo/service/server.py", line 1333, in preload_registries
    registry = Registry.new(dbname, update_module=update_module)
  File "", line 2, in new
  File "/opt/odoo/odoo/tools/func.py", line 87, in locked
    return func(inst, *args, **kwargs)
  File "/opt/odoo/odoo/modules/registry.py", line 87, in new
    odoo.modules.load_modules(registry, force_demo, status, update_module)
  File "/opt/odoo/odoo/modules/loading.py", line 493, in load_modules
    processed_modules += load_marked_modules(cr, graph,
  File "/opt/odoo/odoo/modules/loading.py", line 374, in load_marked_modules
    loaded, processed = load_module_graph(
  File "/opt/odoo/odoo/modules/loading.py", line 190, in load_module_graph
    load_openerp_module(package.name)
  File "/opt/odoo/odoo/modules/module.py", line 471, in load_openerp_module
    __import__('odoo.addons.' + module_name)
  File "/__w/rest-framework/rest-framework/graphql_base/__init__.py", line 4, in 
    from .controllers import GraphQLControllerMixin
  File "/__w/rest-framework/rest-framework/graphql_base/controllers/__init__.py", line 1, in 
    from .main import GraphQLControllerMixin
  File "/__w/rest-framework/rest-framework/graphql_base/controllers/main.py", line 6, in 
    from graphql_server import (
ImportError: cannot import name 'HttpQueryError' from 'graphql_server' (/opt/odoo-venv/lib/python3.10/site-packages/graphql_server/__init__.py)

SirPyTech

Thanks for the review!
Please check the latest changes

SirPyTech · 2025-08-21T09:07:51Z

+        try:
+            headers_dict = {key: value for key, value in headers.items()}
+            return self._sanitize_headers_dict(headers_dict)
+        except AttributeError:


I'm suspicious about the headers argument, if it does not have the items method then an AttributeError would be raised.
But you're right, let's be optimistic and remove the exception handling 😄

SirPyTech · 2025-08-21T09:15:34Z

+    def _prepare_log_response(self, response):
+        return {
+            "response_status_code": response.status_code,
+            "response_headers": self._headers_to_dict(response.headers),


I have added the identifier (api.log.id) of the log record, that is enough to find the log record to whoever has access to them.

Please keep in mind that I agree with the author of #501 that wrote:

This is not a migration of rest_log, this module aims to be simpler, at least in the first iterations.

A more elaborated feature (like building the full URL of the log record) can be added later, after the first simple module is merged.

SirPyTech · 2025-08-21T09:15:44Z

+    def _compute_log_ids(self):
+        for collection in self:
+            collection.log_ids = self.env["api.log"].search(
+                [("collection_ref", "=", "%s,%s" % (collection._name, collection.id))]


SirPyTech · 2025-08-21T09:16:51Z

+from odoo import api, models
+
+
+class FastapiLog(models.Model):


Nice catch! Thanks

paradoxxxzero

Good work thanks. Nothing blocking except the exception commit problem. Once it’s fixed I’ll close my old PR.

paradoxxxzero · 2025-09-04T10:23:11Z

+
+    def _prepare_log_response(self, response):
+        headers_dict = self._headers_to_dict(response.headers)
+        headers_dict["API_LOG_ENTRY_ID"] = self.id


This does not add the id to the headers of the response, only in the logs. Set it in the response instead.

Right, thanks for catching that!
I have added a quick assert in the tests too

paradoxxxzero · 2025-09-04T10:25:33Z

+                            <field name="response_status_code" />
+                            <field name="response_headers_preview" />
+                            <field
+                                name="response_preview"


There is no way to access the full response in the view anymore since the removal of the b64 field.
I think it should be kept one way or another.

👍 Answered to the same query in #554 (comment).

paradoxxxzero · 2025-09-04T10:43:01Z

+                    _logger.warning("Failed to log exception", exc_info=e)
+                else:
+                    # Be sure to commit/save the exception's log
+                    env.cr.commit()


This is very wrong. You are committing a transaction of a failed request.

Consider this simple case :

@router.post("/create_partner") def create_partner(env: Annotated[api.Environment, Depends(optionally_authenticated_partner_env)]): partner = env["res.partner"].create({...}) if is_something_wrong_with_new_partner(partner): raise UserError("Something is wrong with new partner") return "Ok"

If logging is enabled the wrong partner will be stored in database.

Please restore the separate cursor (https://github.com/OCA/rest-framework/pull/501/files#diff-67e6f9ad0c4d04c8de3dcd507c53fa5bbebf76b169c660d976f36c84c236ea68R38) or find another way to commit the exception.

I verified this and you are correct.
I added the separate cursor in a commit with you as a co-author to fix this, could you please check?

SirPyTech

Thanks for the review @paradoxxxzero, please check the latest changes

SirPyTech · 2025-09-09T10:20:36Z

+
+    def _prepare_log_response(self, response):
+        headers_dict = self._headers_to_dict(response.headers)
+        headers_dict["API_LOG_ENTRY_ID"] = self.id


Right, thanks for catching that!
I have added a quick assert in the tests too

SirPyTech · 2025-09-09T10:26:47Z

+                    _logger.warning("Failed to log exception", exc_info=e)
+                else:
+                    # Be sure to commit/save the exception's log
+                    env.cr.commit()


I verified this and you are correct.
I added the separate cursor in a commit with you as a co-author to fix this, could you please check?

SirPyTech · 2025-09-09T10:31:59Z

+                            <field name="response_status_code" />
+                            <field name="response_headers_preview" />
+                            <field
+                                name="response_preview"


👍 Answered to the same query in #554 (comment).

paradoxxxzero

Except for missing header in exception, LGTM

paradoxxxzero · 2025-09-18T10:09:20Z

+        return self.sudo().create(log_request_values)
+
+    def _prepare_log_response(self, response):
+        response.headers["API_LOG_ENTRY_ID"] = self.id


I think it could be better to use kebab-case here

Right, it is the common convention for headers keys but I didn't know that, thanks!

paradoxxxzero · 2025-09-18T10:09:51Z

+        return self.sudo().write(log_response_values)
+
+    def _prepare_log_exception(self, exception):
+        values = {


API_LOG_ENTRY_ID header is not set in case of exception

Thanks, I added it, please check.

Note that it won't be returned in the response to the client but only stored in the log record, because in case of an exception the dispatcher simply propagates the exception raised by the endpoint.

paradoxxxzero · 2025-10-20T10:34:52Z

+        values = {
+            "stack_trace": "".join(format_exception(exception)),
+            "response_headers": self._inject_log_entry(dict()),


I think it is especially important to return the header in case of exception, luckily we can do this by setting exception headers attribute, see: https://github.com/OCA/rest-framework/blob/18.0/fastapi/fastapi_dispatcher.py#L46

Suggested change

values = {

"stack_trace": "".join(format_exception(exception)),

"response_headers": self._inject_log_entry(dict()),

exception.headers = getattr(exception, "headers", {})

values = {

"stack_trace": "".join(format_exception(exception)),

"response_headers": self._inject_log_entry(exception.headers),

}

I agree, that's why I mentioned it specifically, but I didn't know a way to inject it; the solution you proposed is awesome and works great!
I included it, please check the new changes.

paradoxxxzero

LGTM

Also please rename this PR to something like: [16.0][ADD] fastapi_log

SirPyTech · 2025-11-26T09:00:22Z

Rebased to check if this is affected by fastapi/fastapi@51ad909.
It still looks good 😎

paradoxxxzero · 2026-01-13T16:43:43Z

@simahawk @lmignon Can we move forward on this PR? I would like to migrate these modules to 18.0

lmignon · 2026-01-13T17:29:26Z

I don't use this addon. @simahawk as you made a review, is-it ok for you?

claudio-mano · 2026-03-18T14:21:56Z

Hi, I'm currently trying this module, and I found a bug I'd want to report.

Describe the bug

Endpoint raises an exception which has unwritable headers property (in my case oauthlib.oauth2.rfc6749.errors.MismatchingRedirectURIError)
fastapi_log tries to log exception
api_log._prepare_log_exception currently assigns exception.headers = getattr(exception, "headers", {})
Headers is a read-only property, so this triggers:
AttributeError: property headers of MismatchingRedirectURIError object has no setter

This is my traceback
oauthlib.oauth2.rfc6749.errors.MismatchingRedirectURIError: (invalid_request) Mismatching redirect URI. <oauthlib.Request SANITIZED>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/home/claudio/venv-enterprise/addons/extra/fastapi_log/fastapi_dispatcher.py", line 66, in dispatch
log and log.log_exception(response_exc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/claudio/venv-enterprise/addons/extra/api_log/models/api_log.py", line 202, in log_exception
log_exception_values = self._prepare_log_exception(exception)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/claudio/venv-enterprise/addons/extra/fastapi_log/models/api_log.py", line 56, in _prepare_log_exception
values = super()._prepare_log_exception(exception)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/claudio/venv-enterprise/addons/extra/api_log/models/api_log.py", line 175, in _prepare_log_exception
exception.headers = getattr(exception, "headers", {})
^^^^^^^^^^^^^^^^^
AttributeError: property 'headers' of 'MismatchingRedirectURIError' object has no setter

The line of the error is this one
https://github.com/PyTech-SRL/rest-framework/blob/0fae517e7f93f2b2c65505bd9b91f2bc43240a25/api_log/models/api_log.py#L175

SirPyTech · 2026-03-19T11:44:40Z

@claudio-mano thanks for the detailed report in #554 (comment), it helped a lot! I added a commit to fix your use-case, could you please check if it works as expected now?

Hi @SirPyTech, thank you very much for the quick response, I confirm that now everything works fine

Thanks for the feedback, would you mind dropping an approval in this PR? This would speed up the merge process.
If you don't know how to do that, look at the last step of https://odoo-community.org/resources/review:

Once done, do not forget to leave a review on GitHub (accessible in the "Review changes" green button on the third tab - Files changed -) with:

Approve if everything worked as expected

Comment or Request changes with the description of the mismatch otherwise.

Anyone having a GitHub account can post a review.

claudio-mano

Tested my scenario described here: #554 (comment)

This way other APIs might use the new module `api_log` to store logs.

Co-authored-by: Florian Mounier <paradoxxx.zero@gmail.com>

lmignon · 2026-03-26T10:22:14Z

@SirPyTech @paradoxxxzero Is this PR ready to merge?

SirPyTech · 2026-03-26T10:30:03Z

@SirPyTech @paradoxxxzero Is this PR ready to merge?

Thanks @lmignon for taking care of this PR!
Yes I fixed a small regression caught by the unit tests, caused by the latest changes.

I am probably biased because I'm the author of several commits, but in my opinion this could be merged 🚀

lmignon · 2026-03-26T10:32:52Z

/ocabot merge nobump

OCA-git-bot · 2026-03-26T10:32:58Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-554-by-lmignon-bump-nobump, awaiting test results.

OCA-git-bot · 2026-03-26T10:40:06Z

Congratulations, your PR was merged at 5de4d3a. Thanks a lot for contributing to OCA. ❤️

SirPyTech marked this pull request as ready for review August 4, 2025 10:13

SirPyTech mentioned this pull request Aug 4, 2025

Split logging features of fastapi_log akretion/rest-framework#8

Closed

1 task

simahawk requested changes Aug 12, 2025

View reviewed changes

SirPyTech force-pushed the 16.0-add-fastapi_log branch from 7674bd7 to d1f3eec Compare August 21, 2025 10:21

SirPyTech commented Aug 21, 2025

View reviewed changes

SirPyTech requested a review from simahawk August 21, 2025 10:28

sebastienbeau added this to the 16.0 milestone Sep 1, 2025

paradoxxxzero suggested changes Sep 4, 2025

View reviewed changes

SirPyTech force-pushed the 16.0-add-fastapi_log branch from d1f3eec to 4e1d2b8 Compare September 9, 2025 11:01

SirPyTech commented Sep 9, 2025

View reviewed changes

SirPyTech force-pushed the 16.0-add-fastapi_log branch from 4e1d2b8 to d7bf717 Compare September 9, 2025 11:05

HekkiMelody approved these changes Sep 10, 2025

View reviewed changes

SirPyTech requested a review from paradoxxxzero September 10, 2025 06:59

paradoxxxzero approved these changes Sep 18, 2025

View reviewed changes

paradoxxxzero mentioned this pull request Sep 18, 2025

[16.0] [ADD] fastapi_log #501

Closed

SirPyTech force-pushed the 16.0-add-fastapi_log branch from d7bf717 to fb9fad2 Compare September 24, 2025 07:43

SirPyTech requested a review from paradoxxxzero September 24, 2025 07:56

paradoxxxzero suggested changes Oct 20, 2025

View reviewed changes

SirPyTech force-pushed the 16.0-add-fastapi_log branch from fb9fad2 to 67660fd Compare October 20, 2025 10:57

SirPyTech requested a review from paradoxxxzero October 20, 2025 11:00

paradoxxxzero approved these changes Oct 20, 2025

View reviewed changes

SirPyTech changed the title ~~Log FastAPI requests~~ [16.0][ADD] fastapi_log Oct 20, 2025

SirPyTech force-pushed the 16.0-add-fastapi_log branch from 67660fd to 0fae517 Compare November 26, 2025 08:54

paradoxxxzero mentioned this pull request Jan 14, 2026

[18.0][ADD] fastapi_log #584

Open

claudio-mano approved these changes Mar 19, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

paradoxxxzero and others added 14 commits March 25, 2026 09:15

[ADD] fastapi_log

a9c1ab6

[REF] fastapi_log: Extract common features to api_log

1dc6399

This way other APIs might use the new module `api_log` to store logs.

[ADD] fastapi_log_mail

7f961a9

[FIX] fastapi_log: Manage multi-slash endpoints

3609f68

[IMP] api_log: Hide sensitive headers

083f5c0

[IMP] api_log: Add collection of logs

d08b40f

[ADD] api_log_mail: Notify user about logged exceptions

37c8de4

[IMP] fastapi_log: Adapt to log collection

7355602

[IMP] fastapi_log_mail: Adapt to log collection

7db267d

[IMP] api_log: Inject Log record identifier

3171c9f

[IMP] fastapi_log: Use dedicated cursor for logs

6c06e4b

Co-authored-by: Florian Mounier <paradoxxx.zero@gmail.com>

[IMP] fastapi_log_mail: Use dedicated cursor in tests

e5bfb51

[IMP] fastapi_log: In tests, check Log identifier in response

761d710

[FIX] api_log: Name of empty log entry

4bd47a7

SirPyTech force-pushed the 16.0-add-fastapi_log branch from 3217ffc to 281e0ee Compare March 25, 2026 08:15

[FIX] api_log: Log readonly exception

3fd6a4e

SirPyTech force-pushed the 16.0-add-fastapi_log branch from 281e0ee to 3fd6a4e Compare March 26, 2026 10:14

OCA-git-bot added the bot is merging ⏳ label Mar 26, 2026

OCA-git-bot merged commit 3dfc28f into OCA:16.0 Mar 26, 2026
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Mar 26, 2026

SirPyTech mentioned this pull request Mar 27, 2026

[MIG] api_log, api_log_mail, fastapi_log, fastapi_log_mail: Migration to 17.0 #605

Open

Uh oh!

Conversation

SirPyTech commented Aug 4, 2025

Uh oh!

SirPyTech commented Aug 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirPyTech commented Aug 21, 2025

Uh oh!

SirPyTech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paradoxxxzero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirPyTech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paradoxxxzero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

SirPyTech commented Mar 19, 2026 •

edited

Loading