Skip to content

fix(rest): clear error when response_json_field matches a non-text response value#1892

Open
xr843 wants to merge 3 commits into
NVIDIA:mainfrom
xr843:fix/rest-generator-dict-response-error
Open

fix(rest): clear error when response_json_field matches a non-text response value#1892
xr843 wants to merge 3 commits into
NVIDIA:mainfrom
xr843:fix/rest-generator-dict-response-error

Conversation

@xr843

@xr843 xr843 commented Jun 27, 2026

Copy link
Copy Markdown

Fixes #1888 (also addresses the list-shaped variant in #319).

RestGenerator wrapped whatever response_json_field resolved to in a Message without checking its type. When an endpoint's JSON response shape differs from the configured field (e.g. Azure AI Foundry, where the field is a nested object), a dict/list got stored as the Message text. This only surfaced much later in the detector as an opaque 'dict' object has no attribute 'lower' AttributeError — after the whole probe had already run and spent API calls — giving no hint about the root cause (as the reporter noted).

This validates that an extracted value is text (or None) before building a Message and raises an actionable BadGeneratorException naming response_json_field and showing the offending structure. The same clear error is raised when the field is missing/unsubscriptable (previously a bare KeyError) and when a single JSONPath match resolves to a non-text object. Net effect: fail fast at generation time with guidance, instead of crashing opaquely during detection.

Tests added covering dict-valued plain field (#1888 repro), dict-valued JSONPath, and a missing field. All 35 REST generator tests pass; the 3 new tests fail pre-fix and pass post-fix.

RestGenerator wrapped whatever response_json_field resolved to in a
Message without checking its type. When the endpoint's JSON response
shape differed from the configured field (e.g. an Azure-style response
where the field is a nested object), a dict/list was stored as the
Message text. This surfaced much later in the detector as an opaque
"'dict' object has no attribute 'lower'" AttributeError after the whole
probe had already run, giving the user no hint about the root cause.

Validate that an extracted value is text (or None) before building a
Message and raise a clear, actionable BadGeneratorException naming
response_json_field and showing the offending structure. Also raise the
same clear error when the field is missing/unsubscriptable (previously a
bare KeyError/TypeError) and when a single JSONPath match resolves to a
non-text object. This fails fast at generation time instead of crashing
opaquely during detection.

Fixes NVIDIA#1888 (also addresses the list-shaped variant in NVIDIA#319).

Signed-off-by: xr843 <xianren843@protonmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@jmartin-tech jmartin-tech left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The core issue is valid, however I am not sure raising an unrecoverable error is the right answer for a single request failure.

Comment thread garak/generators/rest.py Outdated
# detectors rather than an actionable configuration error. See #1888.
for value in response:
if value is not None and not isinstance(value, str):
raise BadGeneratorException(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

BadGeneratorException will terminate the run, this may need to either raise a retry or return None to indicate a request failure that may be related to the specific request.

There might be some value in detecting if this was the first failure of this type or if any successful requests completed before hitting this error to better decide if the generator is bad or if is this request produces a bad response.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 04e97fa — this path now logs an actionable error (naming response_json_field and the offending type/shape) and returns [None], so a single mismatched response is recorded as a failed generation and the run continues, mirroring the existing "JSONPath yielded nothing" branch just below.

On distinguishing a bad generator from a bad request: I checked #1888's expected behaviour — the reporter's endpoint returns a non-text object for every response, and they still expect the run to "finish and present the report". Gating on "first failure with no prior success → raise" would regress that exact case, since the very first call would abort. So I went with unconditional log-and-skip rather than success-tracking. If you'd rather the misconfiguration be surfaced more loudly than a per-response log line, I can add a one-time warning on the first occurrence — without ever aborting the run.

@jmartin-tech jmartin-tech Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Gating on "first failure with no prior success → raise" would regress that exact case

While the reporter's expectations are a good data point, they are not always requirements and do not define a regression.

In general it make sense to continue when the error that was reported occurs with some probes or prompts and not others. However, when the user has provided an invalid extraction value and no valid results can be obtained it does not really make sense to continue to spend inference costs for an empty report.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed — you're right that the reporter's expectation isn't a requirement, and "invalid extraction value, no valid results obtainable" is a different case from "this one response is odd". Reworked in 70bb907 to draw exactly that line.

RestGenerator now tracks whether any generation has ever been extracted (_extracted_a_response). On a response whose shape response_json_field cannot address:

  • no extraction has succeeded yetBadGeneratorException. There is no evidence the configured field can ever address this endpoint, so the run can only produce an empty report; fail fast rather than spend inference on it.
  • at least one extraction has succeeded → log the actionable error and return [None]. The field demonstrably works against this endpoint, so the mismatch is a property of this response, and the run finishes.

Both failure sites you flagged (the plain-key KeyError/TypeError path and the non-text type validation) funnel through one _response_extraction_failed() helper, so the two stay consistent. The exception carries the same actionable text the log line did — naming response_json_field and the offending type/shape.

Tests updated accordingly: three cases assert the abort on a fresh generator (dict via plain key, dict via JSONPath, missing key), and two assert the skip-and-continue once a prior call has extracted successfully.

One thing I deliberately left alone: the pre-existing "JSONPath yielded nothing" branch just below still returns [None] unconditionally. By the same argument it's also an un-addressable field and could route through the helper — but it's existing behaviour outside this fix, so I didn't want to change it uninvited. Happy to fold it in if you'd like.

Comment thread garak/generators/rest.py Outdated
else:
response = [response_object[self.response_json_field]]
except (KeyError, TypeError) as e:
raise BadGeneratorException(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as other comment, this exception would terminate the run, is that the correct behavior here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Same treatment here in 04e97fa: the type-validation loop now logs the actionable error and returns [None] instead of raising, so a non-text match skips that generation rather than terminating the run.

xr843 added a commit to xr843/garak that referenced this pull request Jul 7, 2026
…ing the run

Per review on NVIDIA#1892: raising BadGeneratorException on a single
response_json_field mismatch terminates the whole run, which is too
aggressive and contradicts the behaviour NVIDIA#1888 asks for ("it should
instead finish and present the report"). In that report the endpoint
returns a non-text object for every response, yet the run is expected to
complete and produce a report.

Log a clear, actionable error naming response_json_field and the
offending type/shape, then return [None] so the generation is recorded as
a failure and the run continues -- mirroring the existing "JSONPath
yielded nothing" path in the same method. This still removes the opaque
downstream "'dict' object has no attribute 'lower'" crash from NVIDIA#1888
without ending the run early.

Update the three tests to assert [None] is returned and the error is
logged, rather than expecting BadGeneratorException.

Signed-off-by: xr843 <xianren843@protonmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jmartin-tech jmartin-tech changed the title fix(rest): clear error when response_json_field matches a non-text response value (#1888) fix(rest): clear error when response_json_field matches a non-text response value Jul 8, 2026
xr843 and others added 2 commits July 9, 2026 08:39
…ing the run

Per review on NVIDIA#1892: raising BadGeneratorException on a single
response_json_field mismatch terminates the whole run, which is too
aggressive and contradicts the behaviour NVIDIA#1888 asks for ("it should
instead finish and present the report"). In that report the endpoint
returns a non-text object for every response, yet the run is expected to
complete and produce a report.

Log a clear, actionable error naming response_json_field and the
offending type/shape, then return [None] so the generation is recorded as
a failure and the run continues -- mirroring the existing "JSONPath
yielded nothing" path in the same method. This still removes the opaque
downstream "'dict' object has no attribute 'lower'" crash from NVIDIA#1888
without ending the run early.

Update the three tests to assert [None] is returned and the error is
logged, rather than expecting BadGeneratorException.

Signed-off-by: xr843 <xianren843@protonmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Track whether a generation has ever been extracted. Before the first
success, a response whose shape response_json_field cannot address means
the configured field cannot address this endpoint at all: raise
BadGeneratorException rather than spend inference on a run that can only
produce an empty report. After the first success the same mismatch is a
property of that response rather than of the configuration, so log it and
skip the generation, letting the run finish and present a report.

Signed-off-by: xr843 <xianren843@protonmail.com>
@xr843 xr843 force-pushed the fix/rest-generator-dict-response-error branch from 04e97fa to 70bb907 Compare July 9, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AttributeError: 'dict' object has no attribute 'lower' on REST endpoint after any probe finish

2 participants