Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ Solved ] Body is unusable. #76

Closed
stephen776 opened this issue Sep 18, 2024 · 15 comments
Closed

[ Solved ] Body is unusable. #76

stephen776 opened this issue Sep 18, 2024 · 15 comments

Comments

@stephen776
Copy link

Hello,

I am currently using the conform example to setup my project but I receive a “Body is unusable” error when submitting the email login form.

my login page is copied directly from the example. My understanding is that this occurs when reading the request body more than once without cloning it.

I’m using node 20/express with single fetch enabled for reference.

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

Hello @stephen776, it's been a while since I looked into remix-auth-totp and Conform, so right now I can't say much.

  • Can you share an example of your use case, or is it simply the example template without any modifications?

Have you tried to clone the request in case that could simply make it work for you?

@dev-xo dev-xo changed the title Body is unusable [ Conform ] Body is unusable. Sep 18, 2024
@stephen776
Copy link
Author

stephen776 commented Sep 18, 2024

Ya I don’t think it’s specifically related to conform. I just copied the login page from the example.

I have not had a chance to look at your source code yet but I am assuming that the strategy is accessing the request body twice somewhere. I am not touching the request anywhere else in the action.

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

Hmm, I see. Feel free to play a bit with it since it's been really a long time since I had the chance to look into it.

Not a big fan myself of supporting some other packages/modules rather than the basic Strategy implementation, as that would probably require some maintenance from time to time.

Feel free to play a bit with it and share some feedback in case you are able to, otherwise I will try to find a moment to look into it.

@stephen776
Copy link
Author

Cloning did not resolve it either.

My action for reference.

image

@stephen776
Copy link
Author

Not a big fan myself of supporting some other packages/modules rather than the basic Strategy implementation, as that would probably require some maintenance from time to time.

Sorry - I don't think I am following - I believe the error is occurring inside the implementation of the strategy you have created for this package. I am happy to provide info to help you debug if you are able to support.

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

Just trying to say that supporting examples like Conform could get me in situations where I will need to spend some time debugging things that may or may not be directly related to the Strategy remix-auth-totp.

In this case, you think it could be related to remix-auth-totp as far as I understand, and if that's the case, we should for sure look into it.

If related to Conform, probably looking into the docs could be better (although I think we have a previous version or one that's not updated in the Example).

In order for me to properly give a response, I would need to be able to test or play a bit with the current Conform implementation; otherwise, it will be hard.


Are you on the provided versions of remix-auth-totp and conform from the template Example, or have those versions been upgraded?

@stephen776
Copy link
Author

stephen776 commented Sep 18, 2024 via email

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

I would suggest simply using the Strategy as it is intended without any extra packages, and in case you need extra validation, handle that directly in the action whenever required.

But yeah, feel free to play a bit with Conform, and let me know if you find something useful in case we can also update the Conform example, helping someone else.

@stephen776
Copy link
Author

stephen776 commented Sep 18, 2024

Ok - I have modified my implementations to use the basic example (no Conform at all) and I am still getting the error on submit.

The error comes back in the authError field of the loader so it's coming from the cookie set by the strategy.

image

This is definitely NOT related to conform. The only other difference is that I am using single-fetch which uses Undici under the hood. see issue

@stephen776
Copy link
Author

Ok - I have traced it further. The issue is in my own sendEmail implementation. It was just hidden by the fact that the error came from the remix-auth cookie. Sorry for the confusion!

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

Interesting, happy you found the solution @stephen776. Also happy to know the Strategy works as expected. Let me know if you find anything or if you have any suggestions we could apply in order to improve the Strategy itself!

@dev-xo dev-xo changed the title [ Conform ] Body is unusable. [ Solved ] Body is unusable. Sep 18, 2024
@stephen776
Copy link
Author

stephen776 commented Sep 18, 2024

It would be really helpful if the error messaging could indicate that the problem occurred within the sendTOTP implementation. As it stands, the true error was essentially swallowed in the catch block of the strategy and return only the (less than helpful) error string.

I tracked this down by adding my own logging to the stategy locally.

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

If you think "Body is unusable" will always be returned from sendTOTP or related to that method, we could for sure include a better error message, @stephen776!

Feel free to open a PR with the changes you think may be worth having, especially if you think someone else will encounter the issue you had sooner or later!

@stephen776
Copy link
Author

stephen776 commented Sep 18, 2024

No it definitely won't always be that particular error. Since sendTOTP is implemented by the consumer, anything can go wrong.

I think it would just be nice to know at least that the error originated in my code. This fact was essentially hidden from me during normal testing.

Maybe as simple as prepending some context to the error message

sendTOTP Error: <whatever the error message is>

@dev-xo
Copy link
Owner

dev-xo commented Sep 18, 2024

I think it would just be nice to know at least that the error originated in my code. This fact was essentially hidden from me during normal testing.

Yeah, I think that would be great. Also, prepending some context, as it could be the method name or similar, could definitely help.

Not sure if you have the time to look into it, otherwise we could let this pending for another time, as having some useful logs here and there could avoid stressing or not knowing where the issue came from (as it was your case).

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

No branches or pull requests

2 participants