aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.43k stars 2.13k forks source link

Error message not available to signInWithRedirect_failure in Hub #13842

Closed johnf closed 1 month ago

johnf commented 1 month ago

Before opening, please confirm:

JavaScript Framework

Next.js

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

Amplify CLI

Environment information

``` # Put output below this line System: OS: Linux 6.8 Ubuntu 24.04.1 LTS 24.04.1 LTS (Noble Numbat) CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor Memory: 13.54 GB / 30.50 GB Container: Yes Shell: 5.2.21 - /bin/bash Binaries: Node: 20.17.0 - ~/.nvm/versions/node/v20.17.0/bin/node Yarn: 1.22.22 - ~/work/gladly/mobile/node_modules/.bin/yarn npm: 10.8.3 - ~/work/gladly/mobile/node_modules/.bin/npm bun: 1.0.30 - ~/.bun/bin/bun Watchman: 4.9.0 - /usr/bin/watchman Browsers: Chrome: 129.0.6668.58 npmGlobalPackages: @aws-amplify/cli: 12.12.6 corepack: 0.29.3 npm: 10.8.2 ```

Describe the bug

I'm using next.js for authentication with Google

I have Cognito PreSignup trigger that creates a standard account if the user email doesn't exist and then returns an error Similar to what is described in https://github.com/aws-amplify/amplify-cli/issues/4427

This works fine in my React Native app, but on the web the path is different

It hits https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/providers/cognito/utils/oauth/completeOAuthFlow.ts#L39-L44 the error_message gets passed to https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/providers/cognito/utils/oauth/createOAuthError.ts#L8-L17

The message looks like it's being passed into the error but it us surfaced at https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/providers/cognito/utils/oauth/attempt and then on to https://github.com/aws-amplify/amplify-js/blob/main/packages/auth/src/providers/cognito/utils/oauth/handleFailure.ts#L19

The error message itself is lost.

I've fixed with this diff and can do a PR

# Make sure on the web signInWithRedirect_failure gets the message

     if (responseType === 'code') {
         return handleCodeFlow({
             currentUrl,
diff --git a/dist/esm/providers/cognito/utils/oauth/handleFailure.mjs b/dist/esm/providers/cognito/utils/oauth/handleFailure.mjs
index b9e802f6acb045d624caaf020d5f6c2255f5dbf0..ce64d428ca60f0a32af70ecd222315722bd4ed5b 100644
--- a/dist/esm/providers/cognito/utils/oauth/handleFailure.mjs
+++ b/dist/esm/providers/cognito/utils/oauth/handleFailure.mjs
@@ -8,7 +8,7 @@ import { resolveAndClearInflightPromises } from './inflightPromise.mjs';
 const handleFailure = async (error) => {
     resolveAndClearInflightPromises();
     await oAuthStore.clearOAuthInflightData();
-    Hub.dispatch('auth', { event: 'signInWithRedirect_failure', data: { error } }, 'Auth', AMPLIFY_SYMBOL);
+    Hub.dispatch('auth', { event: 'signInWithRedirect_failure', data: { error: { message: error.message, ...error } } }, 'Auth', AMPLIFY_SYMBOL);
 };

 export { handleFailure };

Expected behavior

The error message from the backend exception is accessible in the Hub data

Reproduction steps

I can provide a repro if needed but it's basically

Code Snippet

// Put your code below this line.

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

cwomack commented 1 month ago

@johnf, thanks for opening this issue with so much detail! Can you give a little more context on how you're using the error object? Are you using console.log() or doing it in a different way?

johnf commented 1 month ago

@cwomack It is directly related to the social linking in https://github.com/aws-amplify/amplify-cli/issues/4427. You need to know what the error that was thrown in the trigger so you can perform an action.

I have other use cases where I might throw 'Mobile numbers for your country are banned, please contact support'

Hub.listen('auth', async ({ payload }) => {
  const { event } = payload;

  switch (event) {
    case 'signInWithRedirect_failure': {
      const { error } = payload.data;
      if (!error) {
        Sentry.captureMessage('Unhandled signInWithRedirect_failure event', { extra: { event } });
        console.error('Unhandled signInWithRedirect_failure event', JSON.stringify(payload, null, 2));

        return;
      }

      const linkedMatch = error.message.match(/LINKED_EXTERNAL_USER_([^.]+)/);

      if (linkedMatch) {
        const provider = (linkedMatch[1].charAt(0).toUpperCase() +
          linkedMatch[1].slice(1)) as SignInWithRedirectInput['provider'];

        await handleFederatedSignIn(provider);

        return;
      }

      if (error.message.match(/User is not enabled/)) {
        Toast.show('This account is in the process of being deleted, please contact support', { type: 'error' });

        return;
      }

      Toast.show('Unhandled Error: Please contact support for help.', { type: 'error' });

      Sentry.captureMessage('Unhandled signInWithRedirect_failure event', { extra: { event } });
      console.error('Unhandled signInWithRedirect_failure event', JSON.stringify(payload, null, 2));

      return;
    }
  }
cwomack commented 1 month ago

@johnf, thanks for the reply.

The JSON.stringify method cannot serialize a custom error, but you can instead access the properties of the error object before making the console.error() call. As you've already shown in your suggestions for a PR and code change in the issue description, the error.message is accessible.

This shouldn't require us to make any further code changes or consider this behavior to be a bug at this point.

johnf commented 1 month ago

@cwomack Sorry for wasting time here; I was down a deep rabbit hole yesterday with the current race condition around Hub.listen and oAuth.

Looking at this code with fresh eyes, it's obvious that it's superfluous.

Thanks for your help!

cwomack commented 1 month ago

@johnf, thanks for following up and glad you're unblocked!