Skip to content

Conversation

@timu-jesse-ezell
Copy link
Contributor

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Method started returning dict instead of pydantic object

What is the new behavior?

Return the pydantic object

Additional context

There seems have been a behavior change introduced recently, not sure this was intentional, this method used to return the pydantic model, but here it is returning the json. This is breaking our code after updating. Looks like other methods are still returning the pydantic model, so I think this was a mistake.

@jezell
Copy link

jezell commented Oct 30, 2025

Previous implementation for reference, xform=parse_auth_response causes response to be a pydantic model, new code seemed to rely on manually parsing the model, but then returns the wrong one of the two objects.

    async def exchange_code_for_session(self, params: CodeExchangeParams):
        code_verifier = params.get("code_verifier") or await self._storage.get_item(
            f"{self._storage_key}-code-verifier"
        )
        response = await self._request(
            "POST",
            "token",
            query={"grant_type": "pkce"},
            body={
                "auth_code": params.get("auth_code"),
                "code_verifier": code_verifier,
            },
            redirect_to=params.get("redirect_to"),
            xform=parse_auth_response,
        )
        await self._storage.remove_item(f"{self._storage_key}-code-verifier")
        if response.session:
            await self._save_session(response.session)
            self._notify_all_subscribers("SIGNED_IN", response.session)
        return response
     

@silentworks silentworks changed the title Fix: Return auth_response from exchange_code_for_session instead of response dict fix(auth): Return auth_response from exchange_code_for_session instead of response dict Oct 31, 2025
@coveralls
Copy link

coveralls commented Oct 31, 2025

Pull Request Test Coverage Report for Build 18973010848

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 8 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.559%

Files with Coverage Reduction New Missed Lines %
src/supabase_auth/_async/gotrue_client.py 8 73.69%
Totals Coverage Status
Change from base Build 18952536476: 0.0%
Covered Lines: 8745
Relevant Lines: 9448

💛 - Coveralls

@grdsdev grdsdev changed the title fix(auth): Return auth_response from exchange_code_for_session instead of response dict fix(auth): return auth_response from exchange_code_for_session instead of response dict Oct 31, 2025
@o-santi
Copy link
Contributor

o-santi commented Oct 31, 2025

There seems have been a behavior change introduced recently, not sure this was intentional, this method used to return the pydantic model, but here it is returning the json.

Yes, this was a mistake where the variable was only renamed in some places and not others. It wasn't caught because the function does not declare a return type, and mypy infers Any for functions with no declared return type, instead of the usual None (python/mypy#9413).

Copy link
Contributor

@o-santi o-santi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've included both the missing return type annotations, with also some additional ruff reformatting from the last PR.

@o-santi o-santi merged commit 7159116 into supabase:main Oct 31, 2025
11 checks passed
@o-santi
Copy link
Contributor

o-santi commented Oct 31, 2025

Fixed in v2.23.0.

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.

5 participants