fix: raise SnowflakeForbiddenError for HTTP 403 by hampsterx · Pull Request #22 · hampsterx/snowflake-sql-api

hampsterx · 2026-10-19T20:58:03Z

Summary

A 403 means Snowflake accepted the JWT and refused the request (for example, the SQL API is
not enabled for the account). Callers can now tell that apart from a
malformed request without parsing status_code: HTTP 403 raises SnowflakeForbiddenError.

SnowflakeForbiddenError subclasses SnowflakeRequestError and carries the same
status_code, code, sql_state and statement_handle; exported from the package root
The mapping lives in the shared _interpret, so sync and async clients get it, on submit,
status polls, partition fetches and cancel alike
403 is not in the default retry_statuses, so it fails fast. A custom RetryPolicy that
lists 403 still surfaces SnowflakeRetryError with a plain SnowflakeRequestError cause,
the same as every other retried status
Docs: a 403 section in docs/troubleshooting.md (causes, and that an object-privilege
failure is a 422, not a 403), a pointer in docs/authentication.md, and a
register_error(status=403) example in docs/testing.md. The causes follow the
SQL API reference
response-code table, which names "API not enabled"; the network-policy case is worded
conditionally on the message naming an IP

Docs: updated

Backwards compatibility

Non-breaking. Every except SnowflakeRequestError and isinstance check still matches a
403. Only code testing type(exc) is SnowflakeRequestError sees a difference.

Review

External (Codex and GLM-5.2) on e3b9e33: no findings. Fix commit 6828d2b re-reviewed on
its own delta by both: clean, 1 low test ask applied (385117c).
Exempt from re-review: test-only at 385117c, tests/test_transport.py
Analysis pass on e3b9e33: APPROVE, 1 applied (6828d2b); re-run on 385117c: APPROVE,
nothing to apply.
Deferred from the analysis pass:
- mention tests/test_regression.py:127: route.call_count == 1 does not discriminate
  retry behaviour; the pytest.raises type does (mention)
- mention tests/test_transport.py:169: the untyped exhausted-retry cause is now a tested
  contract, so mapping _retry_error by status later changes this test and the doc line
  together (mention)
- mention snowflake_sql_api/transport.py:307: no test exercises AsyncTransport retry or
  exhaustion; predates this change (mention)
- mention docs/testing.md:78: no test drives register_error(status=403); the regression
  tests guard the mapping it passes through (mention)
Written after review: the Backwards compatibility section, and the summary's link to the SQL
API reference

Changes

 docs/authentication.md          |  6 +++-
 docs/testing.md                 |  3 ++
 docs/troubleshooting.md         | 34 ++++++++++++++++++++++
 snowflake_sql_api/__init__.py   |  2 ++
 snowflake_sql_api/exceptions.py | 10 +++++++
 snowflake_sql_api/transport.py  | 18 +++++++++---
 tests/test_package.py           |  3 ++
 tests/test_regression.py        | 62 +++++++++++++++++++++++++++++++++++++++--
 tests/test_transport.py         | 23 +++++++++++++++
 9 files changed, 153 insertions(+), 8 deletions(-)

Test plan

test_regression_403_raises_forbidden_error and its async twin; both fail with the 403
branch removed
test_custom_policy_retrying_403_chains_plain_request_error; fails when _retry_error
maps 403 to the forbidden type
coverage run -m pytest: 194 passed, 5 skipped, 94% coverage
pre-commit run --all-files clean
CI green on this PR

Summary by CodeRabbit

New Features
- Added a dedicated SnowflakeForbiddenError for HTTP 403 responses, available through the package’s public interface. Existing request-error handlers also catch it.
Documentation
- Expanded troubleshooting guidance to explain how 403 errors differ from authentication failures and SQL errors, describe retry behavior and common causes, and show how to handle the exception. Added examples of raising the error in tests.

- A 403 means Snowflake accepted the JWT and refused the request, which callers want to tell apart from a malformed request without parsing status_code. It now raises SnowflakeForbiddenError instead of the generic SnowflakeRequestError. - SnowflakeForbiddenError subclasses SnowflakeRequestError, so every existing except SnowflakeRequestError still catches a 403 (non-breaking). - It carries status_code, code, sql_state and statement_handle like the generic branch, and is exported from the package root. - Sync and async regression tests cover the mapping; troubleshooting and authentication docs gain a 403 section.

- troubleshooting: the default RetryPolicy does not retry a 403; a custom policy listing it surfaces SnowflakeRetryError with a plain SnowflakeRequestError cause. - testing.md: show register_error(status=403) raising SnowflakeForbiddenError. - transport module docstring reflowed to the line length.

…retries 403

coderabbitai · 2026-10-19T20:58:21Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 6c4a961f-5436-4650-831d-c544eece062d

📥 Commits

Reviewing files that changed from the base of the PR and between daf95ba and 385117c.

📒 Files selected for processing: 9

docs/authentication.md
docs/testing.md
docs/troubleshooting.md
snowflake_sql_api/__init__.py
snowflake_sql_api/exceptions.py
snowflake_sql_api/transport.py
tests/test_package.py
tests/test_regression.py
tests/test_transport.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The package now exposes SnowflakeForbiddenError for HTTP 403 responses. The transport includes response details in the exception. Tests and documentation cover synchronous and asynchronous queries, retry behavior, and the distinction between 403 responses and 422 SQL errors.

Changes: HTTP 403 Forbidden Error

Layer / File(s)	Summary
403 exception contract and transport mapping `snowflake_sql_api/exceptions.py`, `snowflake_sql_api/__init__.py`, `snowflake_sql_api/transport.py`	Adds and exports `SnowflakeForbiddenError` as a `SnowflakeRequestError` subclass. The transport maps HTTP 403 responses to this exception with the response message and request-error fields.
Behavior tests and troubleshooting guidance `tests/test_*.py`, `docs/authentication.md`, `docs/testing.md`, `docs/troubleshooting.md`	Adds sync and async regression tests for the exception and its fields, plus a test for custom 403 retry behavior. Documentation describes 403 causes, retry behavior, and the distinction from 422 SQL errors.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SnowflakeAPI
  participant TransportInterpret
  participant Caller
  SnowflakeAPI->>TransportInterpret: Return HTTP 403 response
  TransportInterpret->>Caller: Raise SnowflakeForbiddenError

Merge Risk: ⚪ Minimal · up to 38511

The 403 exception change is ready to merge after normal checks; no actionable issue remains identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: HTTP 403 responses now raise SnowflakeForbiddenError.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

hampsterx added 3 commits October 20, 2026 09:45

test(review): pin the exhausted-retry cause for a custom policy that …

385117c

…retries 403

hampsterx merged commit db747d8 into master Oct 19, 2026
7 checks passed

Navigation Menu

Repository navigation

fix: raise SnowflakeForbiddenError for HTTP 403 - #22

fix: raise SnowflakeForbiddenError for HTTP 403#22
hampsterx merged 3 commits into
masterfrom
fix/forbidden-error

hampsterx commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hampsterx commented Oct 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Backwards compatibility

Review

Changes

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hampsterx commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading