Expose HTTP response headers on FMSException - #251
Open
buptliuhs wants to merge 1 commit into
Open
Conversation
Some API failures are only distinguishable from a response header. QuickBooks Online returns errorCode 003200 with an identical message for both an expired and a revoked access token, and fault.error[].detail is not always populated, so the WWW-Authenticate challenge is what tells the two apart. The correct client behaviour differs sharply: an expired token should be refreshed and retried, while a revoked grant requires the user to re-authorize. The SDK discarded that information. setResponseElements() copied only Content-Encoding, Content-Type and the status line off the response, and ResponseElements had no generic header map, so the header was gone before any exception was constructed. Callers had to bypass DataService and issue raw HTTP to read it. Changes: - ResponseElements holds all response headers in a case-insensitive map. Header names are case-insensitive per RFC 7230 and HTTP/2 lowercases them, so callers must not have to guess. Where a header repeats, the last value is retained, consistent with the existing use of HttpResponse#getLastHeader. - The three connection interceptors populate it. HTTPURLConnectionInterceptor already logged getHeaderFields() and then dropped it. - HandleResponseInterceptor attaches the headers to every FMSException it raises, done once around the existing logic rather than at each of the 13 throw sites so no path is missed. Batch requests are covered too, since IntuitBatchInterceptorProvider inherits the response chain. - FMSException exposes getResponseHeaders() and getResponseHeader(name). getResponseHeaders() returns an empty map rather than null, and both accessors tolerate a null name. All exception subclasses inherit this. Additive only: no signature or behaviour changes, no new dependencies, Java 8 compatible. Known limitation: repeated headers such as Set-Cookie keep only the last value. Adds ResponseHeadersTest, covering header normalisation from both the Apache and HttpURLConnection sources, enrichment on the fault and status-code paths, and null safety. Requires no credentials or network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Some API failures are only distinguishable from a response header, and the SDK discards all of them.
QuickBooks Online returns
errorCode=003200with a byte-identicalmessagefor both an expired access token and a revoked one. The only body field that distinguishes them isfault.error[].detail, and that field is not always populated — in Production it comes backnullfor a revoked token while Sandbox returns"Token revoked". Support case: https://help.developer.intuit.com/s/case/500TR00001m9I8UYAU/v3-api-faulterrordetail-returned-as-null-in-production-but-populated-in-sandbox-for-401-errorcode003200The
WWW-Authenticateresponse header does carry the distinction reliably in both environments:This matters because the correct client behaviour is opposite in the two cases: an expired token should be refreshed and retried, whereas a revoked grant is unrecoverable and requires the user to re-authorize. A client that cannot tell them apart will either retry forever or discard a connection whose refresh token is still perfectly good.
Today an SDK caller cannot reach that header:
HTTPClientConnectionInterceptor.setResponseElements()copies onlyContent-Encoding,Content-Typeand the status line off theHttpResponse.ResponseElementshas no generic header map, so everything else is gone before any exception is constructed.FMSExceptiontherefore has nothing to expose.The only workaround is to bypass
DataServiceand re-issue the request with a raw HTTP client purely to read one header — for a condition the SDK's own exception hierarchy already models asAuthenticationException.Worth noting
HTTPURLConnectionInterceptoralready logs the full header map (LOG.debug("Response headers:" + httpUrlConnection.getHeaderFields())) and then drops it on the next line.Change
ResponseElementsholds all response headers in a case-insensitive map. Case-insensitivity is required rather than cosmetic: header names are case-insensitive per RFC 7230 and HTTP/2 lowercases them, so callers must not have to guess. Where a header repeats, the last value is retained — consistent with the existing use ofHttpResponse#getLastHeaderfor the two headers already captured.HTTPClientConnectionInterceptor,HTTPBatchClientConnectionInterceptor, andHTTPURLConnectionInterceptor(the last viagetHeaderFields(), whose multi-value entries and null-keyed status line are normalised).HandleResponseInterceptorattaches the headers to everyFMSExceptionit raises. Done once around the existing logic rather than at each of the 13 throw sites, so no path is missed and future ones are covered automatically. Batch requests are included, sinceIntuitBatchInterceptorProviderinherits the response chain and only swaps the request interceptor.FMSExceptionexposesgetResponseHeaders()andgetResponseHeader(name). The map accessor returns an empty map rather thannull, and both tolerate anullname. All subclasses inherit this.Compatibility
Additive only — new fields and methods, no signature or behaviour changes, no new dependencies, Java 8 compatible. Existing callers are unaffected.
Known limitation: repeated headers such as
Set-Cookiekeep only the last value.Map<String, List<String>>would preserve them, but single-value lookups are the use case here and a flat map matches the SDK's existing style. Happy to change this if you'd prefer.Tests
Adds
ResponseHeadersTest(9 tests, TestNG) — no credentials, no network:Header[]andHttpURLConnectiongetHeaderFields()sourcesWWW-Authenticateandwww-authenticateboth resolve)AuthenticationException) and the status-code fallback path (InvalidTokenException)getResponseHeader(null)returns nullIt lands in
com.intuit.ipp.interceptors, whichtestng-devkit.xmlalready includes by package, so no suite changes were needed.Verified with
mvn -pl ipp-v3-java-devkit test-compile surefire:test -Dtest=ResponseHeadersTest(9/9 passing) plus the adjacent self-contained interceptor and exception suites (35/35, no regressions).