#weird behavior with args.*Result after updating to sapphire v5.3.1

1 messages Β· Page 1 of 1 (latest)

devout depot
#

Hello,

I have this code (pls see screenshot) available as [p]base64 cmd, this same setup worked perfectly fine when I was on sapphire 5.2.1 but after updating to v.5.3.1 today weird issues like these are popping up. Did something change in @sapphire/result to introduce this change? because it feels like breaking.

also I tried to check whats changed between @sapphire/result v2.6.6 ... v2.7.1 but it says 465 files changed and trying to browse it to figure out what changed makes chrome on my potato laptop hard crash, so can someone help me figure out around this issue? thanks.

lofty valveBOT
#

To help others find answers, you can mark your question as solved via Right click solution message -> Apps -> βœ… Mark Solution

devout depot
#

for now I downgraded/reverted sapphire to v5.2.1 and @sappire/result to v2.6.6 which fixed the weird issue and cmds work again as usual

north galleon
#

@devout depot you're using a non-documented private field πŸ˜…

#

In 2.7.0 it changed to symbols to prevent accidentally accessing private fields

#

However, fixing it is not hard! There are some patterns you're using that can be simplified :)

#

For example getMode can be this:```ts
const getModeResult = await args.pickResult('enum', { enum: nodes });
const getMode = getModeResult.unwrapOr('decode');

#

Keep in mind that @sapphire/result is a library that has nearly all the features from Rust's Option<T> and Result<T, E>, which are very powerful on their own, so you should have everything you need without having to give up safety by accessing the raw data :)

devout depot
north galleon
#

unwrapOr never throws

#

Same for unwrapOrElse

devout depot
#

the thing is I'm not from rust background so I still find using both args.pick etc and args.pickResult bit difficult to use, there is lack of sections/examples for args.*Result in sapphirejs.dev guide area makes it difficult for someone not as proficient as me

but as far as I remember from my early days here back when we had sapphire support in #old-sapphire-support , I used to struggle between which to choose between args.pick/rest vs args.*Result and the former never worked out for me due to so many ways I handle arguments in code, partly because lack of examples for it in the guide and partly because of my inability to understand how it works, so I had settled with args.*Result method since then and have used it everywhere because that feels more readable and fits with how I want to handle all sorts of arguments and atleast args.*Result was shown as one of the recommended ways back then in old support channel as rough examples from what I remember.

now that I find out the said method I was using was non-documented private field and disallowed in 2.7.0, it kinda feels like semi breaking change and if this were mentioned in #Announcements it would have been greatly helpful to someone like me.

To explain what I'm trying to say here, let me take the example you showed above: ```js
const getModeResult = await args.pickResult('enum', { enum: nodes });

what if a user inputs an invalid/unexpected/trollish argument here or let's say doesn't provide argument here, does it raise any error? if yes then do I check via `isErr` first then inform user? like how I'm doing currently or use `args.pick(...).catch(() => ...)` route? but that feels unpleasant UX wise, setting default can, at times, feel unwise than to parse/validate argument and inform user if the input doesn't match the expectation. Just trying to help you explain why I use `args.*Result` since so long.
#

sorry for long wall

#

so if I have to remain up-to-date with sapphire then I'll have to change this in so many places in 2+ years old code which feels huge pain and stressful WhenDespairGetsAtYou
or I stay pinned on sapphire 5.2.1 forever and not have to deal with this unprepared change which kinda feels like breaking

north galleon
# devout depot so if I have to remain up-to-date with sapphire then I'll have to change this in...

I can understand the feeling, my main app, @frosty void, is ridden with legacy code that just repels me. When she was a multipurpose app back then, a single change, even technically non-breaking, was breaking Skyra. It felt like this comic all the time: https://xkcd.com/1172/

devout depot
#

thank you for the kind words, it means a lot sneakhug

north galleon
#

I also understand a lot of people don't have written Rust code. It is after all a very powerful but very complicated to learn and use language. Which is why I focused so much on the documentation, so you can see the methods it has and what you can do with them, also predict the behaviour it will have once you execute it (which to be fair is hard without Result). There are patterns you can follow, for example those codeblocks are similar:

const result = await promiseResultMethod();
const value = result.unwrapOr('test');``````ts
const value = await promiseNativeMethod().catch(() => 'test');``````ts
let value;
try {
  value = resultMethod();
} catch {
  value = 'test';
}```
Or even with `Option`:```ts
const option = await promiseOptionMethod();
const value = option.unwrapOr('test');``````ts
const value = (await promiseOptionMethod()) ?? 'test';```
devout depot
#

sorry to say this πŸ’€ but to my ADHD brain, that try-catch example makes the most sense because it feels more readable , in my head it went like "OHH OMG I can use it like this too? brooo this makes things so much easier to refactor 😭"

#

I think I'll start refactoring my code slowly now, write a helper function to handle args then slap it in all 99 cmds then update to latest sapphire and see how it goes , thank you for all this help luv

#

that try-catch also gives more flexibility on how I want to handle inputs from both success/failure pov so I guess that is why it feels the best choice for me stolen_emoji

north galleon
#

The try-catch is precisely why we have Result, it's bulky otherwise.

#

But like we support both handling your errors with Result, or with try-catch, it's your app, your code is up to you

#

That being said, you made me realise I should add async variations of all the methods, so the following code becomes valid:

#
const mode = (await args.pickResult('enum', { enum: modes }))
  .unwrapOr('decode');

const text = (await (await args.restResult('string', { maximum: 1984, minimum: 2 }))
  .mapErrIntoAsync(() => {
    if (message.type === MessageType.Reply) {
      const ref = await message.fetchReference();
      return ok(ref.embeds.length > 0 ? ref.embeds[0].description : ref.content);
    }

    return err(new UserError('no input given, aborting...'));
  }))
  .unwrapRaw();

// ...```
#

I really need to make AsyncResult and AsyncOption πŸ˜…

glacial sage
north galleon
#

All in all you can keep your code still, with minor changes

glacial sage
#

I noticed that particular part wasn't answered, so I was just filling in the gap

devout depot
#

πŸ‘πŸ½

north galleon
#
- if (getMode.isErr()) getMode = { value: 'decode' };
+ if (getMode.isErr()) getMode = ok('decode');```
#

Same for getText

#

And then on the last lines you dots const mode = getMode.unwrap(); let text = getText.unwrap();

devout depot
#

ohhh I didn't know this can be simplified further like this

#

ablobmindblown me rn

north galleon
#

.unwrap() will throw an error if there's no value tho

#

And needless to say, Sapphire comes with error handlers so UserError is sent to the user

devout depot
north galleon
#

For example Skyra does this a lot:ts const user = args.finished ? message.author : await args.pick('userName'); if (!user.avatar) this.error(LanguageKeys.Commands.Tools.AvatarNone);

#

And that'll send a translated message to the user using the guild's locale, using this code:

#
export function implementSkyraCommandError(identifier: string | UserError, context?: unknown): never {
    throw typeof identifier === 'string' ? new UserError({ identifier, context }) : identifier;
}```
devout depot
#

interesting πŸ‘€

north galleon
#

And Sapphire args always throw an extension of UserError, so if you're not doing dynamic handling nor error message customization, you can do .pick instead

devout depot
#

I have been meaning to extend UserError to customize the error messages that is more understandable to average discord users because with default hard-coded error messages like "There are no more arguments." the feedback I got is alot of users struggle to understand what they need to do, but I keep on forgetting

#

and me coming from old commando js framework where users are still used to typing commands without having to pass any arguments and it sequentially asks the user to interactively input each argument one after other, I have some ideas to also have that type of args handling but that is probably offtopic to the OP so I apologise

thank you for simplifying this for me with the amazing examples you have provided, it genuinely made the refactor so much less stressful, I would give you a hug if we were in IRL

north galleon
devout depot
#

actually I was reading that before opening this thread but the methods without examples went over my head πŸ’€

#

so I said to myself lemme just ask in support forum to make it easier to understand for me πŸ’€πŸ™πŸ½

#

and my dumb adhd brain can't make sense of all assert equal statements in examples

glacial sage
devout depot
#

πŸ‘πŸ½

#

I would want to try contributing better examples if I get time or if my potato brain can formulate a clear visual of it or if it's needed