fix: type ErrorBrowserLocationChangeRequired

Related issue(s)

https://github.com/ory/sdk/issues/277

Type of 'error' should be GenericError instead of ErrorGeneric https://github.com/ory/kratos/blob/53080b0bd7fc3df9c85d87b119e878af7d040232/internal/httpclient/model_error_browser_location_change_required.go#L20

Checklist

• [x] I have read the contributing guidelines.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I am following the contributing code guidelines.
• [x] I have read the security policy.
• [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got the approval (please contact [email protected]) from the maintainers to push the changes.
• [ ] I have added tests that prove my fix is effective or that my feature works.
• [ ] I have added or changed the documentation.

Further Comments

Jul 26 '23 14:07 jossbnd

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (21ab031) 78.33% compared to head (21ab031) 78.33%.

:exclamation: Current head 21ab031 differs from pull request most recent head b637900. Consider uploading reports for the commit b637900 to get more accurate results

@@           Coverage Diff           @@
##           master    #3397   +/-   ##
=======================================
  Coverage   78.33%   78.33%           
=======================================
  Files         346      346           
  Lines       23551    23551           
=======================================
  Hits        18448    18448           
  Misses       3710     3710           
  Partials     1393     1393           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jul 26 '23 15:07 codecov[bot]

I believe this is correct - genericError is a legacy type. What issues are you observing?

not properly an issue, just that the error looks like this: { error: { error: { ... } }, redirect_browser_to: string } }

instead of the other errors:

{ error: { ... }, redirect_browser_to: string } }

meaning I cannot handle it like the others

Aug 18 '23 06:08 jossbnd

Could you please share the full error and the API you used to get it?

Aug 22 '23 13:08 aeneasr

Could you please share the full error and the API you used to get it?

I use the @ory/kratos-client v0.13.1.

Actually, don't know how to trigger this error ErrorBrowserLocationChangeRequired, if you tell me I can try.

But I can just say the structure is not equivalent to other errors as described above. Meaning we need specific code to handle it.

Nov 24 '23 07:11 jossbnd

Could you please share the full error and the API you used to get it?

I trigger the error by submitting a login flow with social login and a header Accept set to application/json. As below:

curl 'https://auth.my-domain.com/kratos/self-service/login?flow=<flow_id>' \
  -H 'authority: auth.my-domain.com' \
  -H 'accept: application/json' \
  -H 'content-type: application/x-www-form-urlencoded' \
  -H 'origin: null' \
  --data-raw 'provider=google' \
  --compressed

The error looks ok but the structure doesn't fit the Model.

Actual response:

{
    redirect_browser_to: 'https://accounts.google.com/o/oauth2/v2/auth',
    error: {
      id: 'browser_location_change_required',
      code: 422,
      status: 'Unprocessable Entity',
      reason:'In order to complete this flow please redirect the browser to: https://accounts.google.com/o/oauth2/v2/auth',
      message: 'browser location change required',
    },
}

Model - https://www.ory.sh/docs/kratos/reference/api#tag/frontend/operation/updateLoginFlow :

{
  "redirect_browser_to": "string",
  "error": {
    "error": {
      "id": "string",
      "code": 422,
      "status": "string"
      "reason": "string",
      "message": "string",
    }
  },
}

Jan 10 '24 10:01 jossbnd

Is there anything blocking this PR being merged? It would be great to have this fixed in the next kratos release.

Jul 11 '24 17:07 brahmlower