Skip to content

fix(login): surface raw response body for non-JSON auth errors (fixes #305)#306

Open
c-reichert wants to merge 1 commit into
foxriver76:masterfrom
c-reichert:fix/login-surface-raw-error-body
Open

fix(login): surface raw response body for non-JSON auth errors (fixes #305)#306
c-reichert wants to merge 1 commit into
foxriver76:masterfrom
c-reichert:fix/login-surface-raw-error-body

Conversation

@c-reichert
Copy link
Copy Markdown

Fixes #305.

Problem

When the Bring backend rejects credentials with a text/plain body — most commonly Invalid Email. for a syntactically-bogus address — resp.json() throws SyntaxError and the existing catch surfaces:

  • Node 20+/Bun: Cannot Login: Failed to parse JSON
  • Node 18: Cannot Login: Unexpected token I in JSON at position 0

The 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 an error field continues to flow through the existing 'error' in data branch, so the upstream email password combination not existing test path is preserved.

async login(): Promise<void> {
    let resp: Response;
    let bodyText: string;

    try {
        resp = await fetch(`${this.url}bringauth`, {
            method: 'POST',
            body: new URLSearchParams({ email: this.mail, password: this.password })
        });
        bodyText = await resp.text();
    } catch (e: any) {
        throw new Error(`Cannot Login: ${e.message}`);
    }

    let data: AuthResponse;
    try {
        data = JSON.parse(bodyText);
    } catch {
        const snippet = bodyText.trim().slice(0, 500) || '(empty body)';
        throw new Error(`Cannot Login: ${resp.status} ${snippet}`);
    }

    if ('error' in data) {
        throw new Error(`Cannot Login: ${data.message}`);
    }
    // …
}

Before / After

// Before, with a syntactically-bogus email:
Cannot Login: Failed to parse JSON

// After:
Cannot Login: 401 Invalid Email.

Tests

npm test (test/testMinimal.js, example@example.com / secret) still passes — that path returns JSON with email password combination not existing, and the 'error' in data branch handles it as before:

  Wrong login test
    ✔ init should take less than 2000ms (542ms)
    ✔ init should throw invalid JWT error (120ms)

  2 passing (666ms)

Scope

This PR only touches login(). The same resp.json()'error' in data pattern in loadLists, 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.js and build/bring.js.map regenerated via npm run build (TS 4.8 / committed tsconfig.build.json), matching how the existing build/ is committed in the repo.

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

login() swallows text/plain auth error messages (e.g. "Invalid Email.") as "Failed to parse JSON"

1 participant