Skip to content

Conversation

pflynn-virtru
Copy link
Member

This pull request makes a targeted fix to the error handling logic in the newGranter function within sdk/tdf.go. The change ensures that if an error occurs while creating the granter, the function immediately returns the error, preventing further operations on a potentially invalid object.

Error handling improvement:

  • Added an early return in the newGranter function to immediately return if err is not nil, improving reliability and preventing assignment to g.keyInfoFetcher when an error occurs. (sdk/tdf.go, sdk/tdf.goR458-R460

@pflynn-virtru pflynn-virtru requested review from a team as code owners September 15, 2025 20:24
@github-actions github-actions bot added the comp:sdk A software development kit, including library, for client applications and inter-service communicati label Sep 15, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @pflynn-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the robustness of the newGranter function by refining its error handling mechanism. The core improvement ensures that any error encountered during the granter's initialization is promptly propagated, thereby safeguarding against the use of partially constructed or invalid objects and improving overall system reliability.

Highlights

  • Improved Error Handling: An early return has been added to the newGranter function. If an error occurs during the granter creation, the function now immediately returns the error, preventing subsequent operations on a potentially invalid object.
  • Preventing Invalid State: This change specifically prevents the g.keyInfoFetcher field from being assigned when an error has already occurred, ensuring the granter object remains in a consistent state.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Error checks now swift, Invalid state avoided, Code runs clean and safe.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a small but important fix to the error handling in the newGranter function. By adding an early return when an error is encountered during granter creation, it prevents further operations on a potentially uninitialized granter object. This improves the robustness and correctness of the code. The change is well-targeted and correct. I have no further comments.

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 173.454768ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 99.07801ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 347.438278ms
Throughput 287.82 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.980291037s
Average Latency 368.225895ms
Throughput 135.21 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.732129109s
Average Latency 256.158804ms
Throughput 194.31 requests/second

elizabethhealy
elizabethhealy previously approved these changes Sep 16, 2025
@dmihalcik-virtru
Copy link
Member

the perils of 'extract function' refactoring

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 182.042323ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 98.85181ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 353.929003ms
Throughput 282.54 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.282738865s
Average Latency 370.998205ms
Throughput 134.11 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.82259808s
Average Latency 257.205798ms
Throughput 193.63 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 176.207932ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 95.458068ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 334.369672ms
Throughput 299.07 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.055047157s
Average Latency 358.865333ms
Throughput 138.68 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.425096755s
Average Latency 253.162071ms
Throughput 196.66 requests/second

@dmihalcik-virtru dmihalcik-virtru added this pull request to the merge queue Sep 17, 2025
Merged via the queue into main with commit a1bebc5 Sep 17, 2025
50 of 52 checks passed
@dmihalcik-virtru dmihalcik-virtru deleted the fix/sdk-newGranter branch September 17, 2025 14:42
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.8.0](sdk/v0.7.0...sdk/v0.8.0)
(2025-09-19)


### Features

* **policy:** obligations + values CRUD
([#2545](#2545))
([c194e35](c194e35))


### Bug Fixes

* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.3.0 to
0.5.0 in /sdk ([#2693](#2693))
([b511048](b511048))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.5.0 to
0.6.0 in /sdk ([#2712](#2712))
([74956bf](74956bf))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.7.0 to
0.8.0 in /sdk ([#2692](#2692))
([fac2ef2](fac2ef2))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.8.0 to
0.9.0 in /sdk ([#2724](#2724))
([e07cc91](e07cc91))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.9.0 to
0.10.0 in /sdk
([#2737](#2737))
([f4a8d1d](f4a8d1d))
* **sdk:** newGranter nil check
([#2729](#2729))
([a1bebc5](a1bebc5))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:sdk A software development kit, including library, for client applications and inter-service communicati size/xs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants