Reclassify OAuthException code 190 as Unauthorized

[craigphares]

Why

Meta returns expired / invalid / revoked access-token errors as an OAuthException with code 190, frequently over HTTP 400 rather than 401. The middleware mapped exceptions purely by HTTP status, so a token error and a genuine bad request both raised BadRequest. Consumers (LinkMyPhotos) could not distinguish them, which caused Media#cleanup to soft-delete live posts on token expiry.

What

• Add an Unauthorized error class (none existed).
• Error now carries http_status / payload and exposes #code / #error_subcode (supports nested {"error":{"code"}} and flat {"code"} envelopes).
• Middleware raises Unauthorized whenever the payload carries code 190 (any status), plus a 401 -> Unauthorized case. Non-token 400s still raise BadRequest. Bump to 0.0.4.

Test

Added spec/raise_http_exception_spec.rb (Faraday test adapter). bundle exec rspec spec/raise_http_exception_spec.rb -> 6 examples, 0 failures.

Note

Verify the exact token-error envelope against a real response: this keys on error.code == 190. If the live Basic Display API returns a different code/shape for token expiry, adjust access_token_error?.

[craigphares]

@claude review this PR

[claude]

Claude finished @craigphares's task in 2m 59s —— View job

---

Review: Reclassify OAuthException code 190 as Unauthorized

• Gather context (diff, changed files)
• Review lib/instagram_basic_display_api/error.rb
• Review lib/instagram_basic_display_api/raise_http_exception.rb
• Review spec/raise_http_exception_spec.rb
• Attempt to run test suite (blocked — see note below)
• Post final review

Summary

Good, focused fix. The core problem — that a token-expiry error and a genuine bad request were indistinguishable because both raised BadRequest on HTTP 400 — is real and the fix addresses it cleanly:

• Error gaining http_status/payload/#code/#error_subcode is a sensible, minimal API addition.
• Splitting exception_class (classification) from error_message (formatting) in raise_http_exception.rb is a nice refactor of what was a single big case statement.
• The new spec (spec/raise_http_exception_spec.rb) covers the important cases: nested vs. flat envelope, 400-with-190 vs. 400-without-190, plain 401, and non-token status codes.
• Bonus fix: HTTP 401 previously raised nothing at all (there was no when 401 branch in the old code) — this PR both adds that handling and routes it to Unauthorized.
• Version bump (0.0.3 → 0.0.4) is consistently applied in both instagram_basic_display_api.gemspec and Gemfile.lock.

Suggestions

1. access_token_error? matches on code == 190 alone, across any HTTP status (lib/instagram_basic_display_api/raise_http_exception.rb:53-55). This is called out as an open question in the PR's own "Note" section, and it's worth taking seriously: Meta's numeric error codes aren't guaranteed unique across error types, so a 500-series response that happens to carry code: 190 for an unrelated reason would now be misclassified as Unauthorized instead of e.g. InternalServerError. Consider additionally checking payload.dig('error', 'type') == 'OAuthException' (or the flat equivalent) to narrow the match. Fix this →

2. Minor inconsistency between #code and #error_subcode (lib/instagram_basic_display_api/error.rb:14-23). #code falls back to a flat payload['code'] when there's no nested error hash, but #error_subcode only ever reads error_hash['error_subcode'] with no flat fallback. Probably fine in practice (subcodes are a Meta-nested-envelope concept), but worth a comment or aligning the two for consistency.

3. Behavioral/breaking change for existing consumers: any caller currently doing rescue InstagramBasicDisplayAPI::BadRequest to catch a 400 that happens to be a token error will now miss it, since Unauthorized is a sibling of BadRequest, not a subclass — which is the whole point of the fix, but worth a line in the gem's README/CHANGELOG (there isn't one currently) so downstream consumers upgrading to 0.0.4 know to add a rescue Unauthorized branch.

4. Could not execute the test suite in this sandbox — bundle exec rspec and even plain shell commands (ruby -v, git fetch) required approval that wasn't available in this run, so I reviewed statically rather than confirming the "6 examples, 0 failures" claim myself. The test file itself (read in full) looks correct and exercises the middleware realistically via Faraday::Adapter::Test. If you want me to actually run the suite in future reviews, this may need bundle exec / rspec added to allowed tools.

Overall: solid, well-tested fix for a real bug. The "match code 190 regardless of status/type" scoping (#1) is the one thing I'd want a second look at before merging, since it's the broadest behavioral change and is explicitly flagged as unverified against a live response in the PR description itself.