Skip to content

Avoid stack inspection in database reader - #506

Open
KonstantinKritsin wants to merge 1 commit into
maxmind:mainfrom
KonstantinKritsin:agent/native-aot-support
Open

Avoid stack inspection in database reader#506
KonstantinKritsin wants to merge 1 commit into
maxmind:mainfrom
KonstantinKritsin:agent/native-aot-support

Conversation

@KonstantinKritsin

@KonstantinKritsin KonstantinKritsin commented Jul 22, 2026

Copy link
Copy Markdown

Avoid stack inspection in database reader

Summary

  • Stop using StackFrame.GetMethod() to construct the unsupported-database
    lookup error.
  • Report the requested database lookup type directly.
  • Document the user-visible error-message change in the release notes.

This removes one trimming warning from the database reader without changing
lookup behavior. It is independent of the larger MaxMind.Db model-mapping
work and the separate source-generated JSON design needed by the web-service
client. The latter is tracked in #505.

Verification

  • 151 tests pass on net8.0.
  • 151 tests pass on net10.0.
  • With IsAotCompatible=true and
    VerifyReferenceAotCompatibility=true, the StackFrame.GetMethod() IL2026
    warning is gone. The five remaining warnings are the known MaxMind.Db
    reference warning and four reflection-based System.Text.Json warnings in
    WebServiceClient.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messages for unsupported database lookups by identifying the requested lookup type.
    • Removed call-stack inspection from this error path, improving compatibility with trimming and Native AOT deployments.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 098d3752-0759-4063-8f6e-37928b34deaf

📥 Commits

Reviewing files that changed from the base of the PR and between 6a94ee5 and 8e1bd0b.

📒 Files selected for processing (2)
  • MaxMind.GeoIP2/DatabaseReader.cs
  • releasenotes.md

📝 Walkthrough

Walkthrough

The unsupported database lookup error path no longer inspects the call stack. Its exception message reports the requested lookup type, and the release notes document the trimming-warning and Native AOT compatibility change.

Changes

Lookup error reporting

Layer / File(s) Summary
Simplify unsupported lookup reporting
MaxMind.GeoIP2/DatabaseReader.cs, releasenotes.md
The invalid lookup path removes caller method extraction, reports the lookup type in the exception, updates the diagnostics import, and documents the change for version 6.1.1.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: oschwald

Poem

A rabbit hops through lookup land,
No stack frames trail behind;
The error names the type it sought,
A trim-safe path to find.
Native AOT smiles softly—
“Clean hops!” the bunny cheered.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing stack inspection from the database reader error path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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

Comment @coderabbitai help to get the list of available commands.

@KonstantinKritsin
KonstantinKritsin marked this pull request as ready for review July 22, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant