fix: handle missing FastAPI request client

[rednikisfun]

Summary

• Handles FastAPI/Starlette requests where request.client is None.
• Returns the library empty value for missing remote IP and port instead of raising AttributeError.
• Adds a regression test for the missing-client case.

Fixes #90

Verification

• python -m pytest tests/test_fastapi.py
• python -m pytest --ignore tests/smoketests
• python -m flake8 json_logging/framework/fastapi/implementation.py tests/test_fastapi.py --count --select=E9,F63,F7,F82 --show-source --statistics

Notes

• The broader CI-style exit-zero flake8 report still shows pre-existing warnings outside this change path.

Summary by CodeRabbit

• Bug Fixes

  • Improved robustness of FastAPI request information extraction to gracefully handle cases where client details are unavailable, preventing logging failures.
  • Remote IP and remote port now return an empty value when request client information is missing.

• Tests

  • Added test coverage for missing client information scenarios to verify remote IP/port fallback behavior.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26b553ef-232f-46f1-afec-03faf8f4808e

📥 Commits

Reviewing files that changed from the base of the PR and between e5e459d and e6d236b.

📒 Files selected for processing (1)

• json_logging/framework/fastapi/implementation.py

🚧 Files skipped from review as they are similar to previous changes (1)

• json_logging/framework/fastapi/implementation.py

---

📝 Walkthrough

Walkthrough

FastAPIRequestInfoExtractor.get_remote_ip and get_remote_port now guard access to request.client before dereferencing .host or .port, returning json_logging.EMPTY_VALUE when client is None. A new test verifies both methods handle a client=None request without error.

Changes

FastAPI client None guard

Layer / File(s)	Summary
Null-guard for request.client with test coverage json_logging/framework/fastapi/implementation.py, tests/test_fastapi.py	get_remote_ip and get_remote_port each add a conditional check on request.client, returning json_logging.EMPTY_VALUE when absent. A new test test_request_info_extractor_handles_missing_client constructs a Request with client=None and asserts both methods return EMPTY_VALUE.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A client of None once caused quite a fright,
With AttributeError appearing at night.
But now a small guard checks if client is there,
And returns EMPTY_VALUE with elegant care.
No more NoneType errors — the rabbit hops right! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: handling missing FastAPI request client by adding null checks to prevent AttributeError.
Linked Issues check	✅ Passed	The PR fully addresses issue #90 by adding null checks in get_remote_ip and get_remote_port methods and returning EMPTY_VALUE when request.client is None, preventing AttributeError exceptions.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the missing request.client handling in FastAPI implementation and adding a regression test, with no out-of-scope modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-code-review]

PR Summary by Qodo

Fix FastAPI extractor when request.client is missing
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Description

• Guard FastAPI/Starlette request client access when request.client is None.
• Return json_logging.EMPTY_VALUE for missing remote IP/port instead of raising AttributeError.
• Add regression coverage for the missing-client request scope case.

Diagram

graph TD
  A["FastAPI/Starlette Request"] --> B["FastAPIRequestInfoExtractor"] --> C{Client present?} --> D["remote ip/port"] --> F["JSON log fields"]
  C --> E["EMPTY_VALUE"] --> F
  G["tests/test_fastapi.py"] --> B

  subgraph Legend
    direction LR
    _req[Request] ~~~ _cls[Extractor] ~~~ _dec{Decision} ~~~ _val[Value]
  end

Loading

High-Level Assessment

The explicit if request.client guard is the most direct and readable fix for Starlette's optional client field, and it aligns with the library’s existing EMPTY_VALUE convention. Alternatives like getattr(request.client, "host", EMPTY_VALUE) or reading from request.scope.get("client") would be slightly more compact but reduce clarity and don’t materially improve correctness.

Files changed (2) +25 / -2

Bug fix (1) +6 / -2

implementation.pyReturn EMPTY_VALUE when request.client is None +6/-2

Return EMPTY_VALUE when request.client is None

• Adds guards in 'get_remote_ip()' and 'get_remote_port()' to handle 'request.client' being 'None'. When missing, the extractor now returns 'json_logging.EMPTY_VALUE' instead of raising 'AttributeError'.

json_logging/framework/fastapi/implementation.py

Tests (1) +19 / -0

test_fastapi.pyAdd regression test for missing request client +19/-0

Add regression test for missing request client

• Introduces a unit test that constructs a FastAPI Request with 'client=None' in the ASGI scope. Verifies the extractor returns 'json_logging.EMPTY_VALUE' for both remote IP and port.

tests/test_fastapi.py

[gemini-code-assist]

Code Review

This pull request updates the FastAPI request info extractor to safely handle missing request client information by returning a default empty value, and adds a corresponding unit test. The review feedback suggests explicitly checking if request.client is not None: instead of relying on truthiness (if request.client:), which aligns with PEP 8 recommendations for checking variables that default to None and prevents potential issues if the client object evaluates to False in a boolean context.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

According to PEP 8, when testing whether a variable that defaults to None was set to some other value, you should explicitly compare it using is not None rather than relying on truthiness. This is especially important for objects that are containers (like NamedTuple / tuple which request.client is), as they could potentially evaluate to False in a boolean context if empty.

Suggested change

	if request.client:
	if request.client is not None:

References

1. PEP 8: Programming Recommendations - Comparisons to singletons like None should always be done with is or is not, never the equality operators. Also, beware of writing if x when you really mean if x is not None. ^(link)

[gemini-code-assist]

According to PEP 8, when testing whether a variable that defaults to None was set to some other value, you should explicitly compare it using is not None rather than relying on truthiness. This is especially important for objects that are containers (like NamedTuple / tuple which request.client is), as they could potentially evaluate to False in a boolean context if empty.

Suggested change

	if request.client:
	if request.client is not None:

References

1. PEP 8: Programming Recommendations - Comparisons to singletons like None should always be done with is or is not, never the equality operators. Also, beware of writing if x when you really mean if x is not None. ^(link)

[llamapreview]

Failed to parse AI response: ``````

[bobbui]

thanks for the contribution