fix(login): surface raw response body for non-JSON auth errors (fixes #305)#306
Open
c-reichert wants to merge 1 commit into
Open
fix(login): surface raw response body for non-JSON auth errors (fixes #305)#306c-reichert wants to merge 1 commit into
c-reichert wants to merge 1 commit into
Conversation
When the Bring backend rejects credentials with a `text/plain` body (e.g. `Invalid Email.`), `resp.json()` throws SyntaxError and the catch block swallows the original message as `Cannot Login: Failed to parse JSON` (Node 20+) or `Cannot Login: Unexpected token I in JSON at position 0` (Node 18). Callers cannot tell wrong-email from wrong-password from rate-limited. Read the body as text first, then attempt JSON.parse. If parsing fails, surface the raw response (status + first 500 chars of body) in the error message. JSON responses with an `error` field still flow through the existing `'error' in data` branch unchanged, so the existing `email password combination not existing` test path keeps working. Fixes foxriver76#305
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.
Fixes #305.
Problem
When the Bring backend rejects credentials with a
text/plainbody — most commonlyInvalid Email.for a syntactically-bogus address —resp.json()throwsSyntaxErrorand the existing catch surfaces:Cannot Login: Failed to parse JSONCannot Login: Unexpected token I in JSON at position 0The actual server message is silently discarded, so callers cannot distinguish wrong-email from wrong-password from a rate-limit response.
Fix
Read the body as text first, then attempt
JSON.parse. If parsing fails, surface the raw response (status + first 500 chars of body) verbatim in the thrown error message. Otherwise behavior is identical to before: a JSON body with anerrorfield continues to flow through the existing'error' in databranch, so the upstreamemail password combination not existingtest path is preserved.Before / After
Tests
npm test(test/testMinimal.js,example@example.com/secret) still passes — that path returns JSON withemail password combination not existing, and the'error' in databranch handles it as before:Scope
This PR only touches
login(). The sameresp.json()→'error' in datapattern inloadLists,getItems,getItemsDetails, etc. has the same failure mode under e.g. rate-limit responses, but those are out of scope here — happy to follow up with a separate PR if you'd like.Build
build/bring.jsandbuild/bring.js.mapregenerated vianpm run build(TS 4.8 / committedtsconfig.build.json), matching how the existingbuild/is committed in the repo.