Talk about Exceptions Here #56365
Comments
I love the irony that a maintainer was forced to check this box 😄 |
|
Would be happy to be involved. I did like the idea of ESLint plugin because it's piggybacking off of an already established static analysis solution, but I think IDE plugins also check that box. This comment has some code for the starting point of the ESLint plugin (in a collapsed text block): #13219 (comment) |
@kvenn see my comment here about eslint michaelangeloio/does-it-throw#70 (comment) I've got jetbrains-intellij working now, but waiting on jetbrains to approve it! You can check the code for that here if you'd like: https://github.com/michaelangeloio/does-it-throw/tree/main/jetbrains |
|
Heck yes! I'll happily use the IntelliJ plugin. I'll check back in a bit and install when it's approved. Shame that eslint doesn't support async and that it's a ways away. But there's even more you can do with a plugin. Nicely done! |
|
@kvenn jetbrains is now available! https://plugins.jetbrains.com/plugin/23434-does-it-throw- Feel free to share with others! |
|
I've since gotten the opportunity to try out the JetBrains plugin for does-it-throw and after looking into it a bit more, I don't think that really solves the problem I'm having with exceptions. That plugin seems to mostly be about alerting of where throw statements are used. Which appears to be for enforcing that you don't use throw statements. I think throw statements are here to stay, even if I agree first class support for errors has advantages. And that if you're already in a codebase which relies on I had proposed an ESLint exception to warn when invoking a function that can throw. Encouraging you to either mark(document) this function as one that re-throws or to catch it. With the intention being to prevent you from accidentally having a function that throws bubble up all the way to the top of your program. But allowing that to be the case if it makes sense (like in a GraphQL resolver, where the only way to notify Apollo of the error is via throwing, or a cloud function / queue where throwing is used to retry). If it can be found implicitly (without documentation), that's better. And it seems like a plugin could actually achieve that (and even offer quick fixes, which would be SO COOL). I'd advocate for using an already used standard TSDoc annotation (@throws) as the acknowledgement that this throw statement is there on purpose (as opposed to introducing a new one - does-it-throw has some great bones. And it seems like it's solving a problem for others, it just might not be the right fit for me. |
|
@kvenn I've tried my hand at an eslint plugin: https://github.com/Kashuab/eslint-plugin-checked-exceptions/tree/main It introduces two rules:
Checks to see if a function you're calling has a
Warns you if a function has a Check out the tests for examples and what it covers. It's been a while since I looked at this, but I remember it being a bit buggy (i.e. nested branches, complicated logic) so there's a ton of room for improvement. I might take another look to improve it. Side note - the README suggests you can install it from NPM, this is not the case haha. (This is my work GH account, dunno why I have a separate one but oh well. I previously contributed here as @KashubaK) |
|
I'm definitely a complete noob when it comes to how Javascript/Typescript works, but would it be possible to add the modifier https://docs.oracle.com/javase/tutorial/essential/exceptions/declaring.html As both a Typescript user and a library maintainer, I hate not being able to consume / deliver proper error declarations. I know I can use JSDoc's |
|
@wiredmatt That suggestion was discussed in detail in the linked issue: #13219 The TL;DR is essentially, it's not worth doing because there isn't sufficient existing practice/documentation/runtime behavior to facilitate such a feature. TypeScript is designed to fit within the scope of JS' behavior, and since JavaScript doesn't give us reliable, native tools for things like checked exceptions it's challenging to fit it within scope. What we're pondering now is, what's the next best thing? How can we encourage better error handling practices enough that the community has some common ground to operate on? |
|
@KashubaK thank you for your response. I was thinking about the newly introduced type guards / type predicates, in my mind it seemed totally possible, especially knowing we have conditional types as well. I'll keep an eye on the eslint solution, that makes sense to me knowing what you just explained. thanks! |
|
I was thinking about actual "throws" keyword as optional in return type, so TS would automatically add defaults to current functions/methods .. something like function doAuth(): AuthPayload throws<TypeError | AuthError> {}and maybe have some utility type similar as typeof to extract errors types from function so we can more easily utilize other modules throw types without directly importing those. function someStuff(): AuthPayload throws<throwof doAuth | FatalError> {}Also maybe this have later impact on catch argument type to actually know throw types, but just actual documentation of throw types is way more important atm for interoperability between modules as currently we are just quessing and reading module source code to undestand what might actually get thrown. Edit: |
This doesn't really work unless you have a level of information that doesn't exist in the real world, and requires the ability to express many patterns that are basically arbitrarily complicated. See #13219 (comment) |
If documentation is the issue, you don't need a TS keyword -- https://jsdoc.app/tags-throws has existed for ages. I don't know about you, but I really don't see it used very often. This is the heart of the problem Ryan described in the comment linked above (summarizing the original issue): JS developers don't, broadly speaking, document expected exception behavior, so there's a chicken and egg problem where trying to implement checked-exception types would go against the grain of the current ecosystem. Use of the All that said, I still think there could be a place for some limited ability to perform static analysis of exception/rejection handling. I originally found the previous issue when I enabled a linter rule that looks for unhandled Promise rejections, which overlaps with try/catch once /** unsafe-assertion-that-this-throws-strings */
function throwsSometimes(): number {
if (Math.random() < 0.5) { throw 'nope!'; }
return (Math.random() < 0.5) ? 0 : 1;
}
/** unsafe-assertion-that-this-throws-never */
function throwsNever(): number { return JSON.parse('2'); }
/** checked-assertion-that-this-throws-never */
function maybeSafe(): number {
return throwsSometimes() || throwsNever(); // error, unhandled throws-strings does not match declared throws-never
}Note that this is a different scope from what was discussed in the checked-exceptions section of #13219 (comment). I'm trying to statically analyze that explicit/declared |
I don't know if this is true. Annotating with A static analysis solution does seem to be the best. And leveraging |
|
How about instead of all this, in your code you just return an function wow(value: unknown) {
if (typeof value === 'string') return new StringNotSupportedError();
return { value: 1234 };
}
const result = wow('haha');
// @ts-expect-error
result.value; // TS error, you have to narrow the type
if (result instanceof Error) {
// Handle the error
return;
}
console.log(result.value); // Good!It forces you to handle errors. Seems pretty similar to what people are asking for. I know that actually I find that the more I think about this, the more I care about it only in my application source code. I'm not all that worried about third party libraries. I don't think there's been a single time where I wished a library had an error documented. Usually good type definitions avoid runtime errors that are worth catching. I also wonder if errors are even suitable for the things I have in mind. Things like validation contain useful state that don't really make sense to wrap in an error, and should instead just be included in a return value. My questions are, what are the real world use-cases here? How do you guys actually see yourselves using a feature like this in practice? What errors do you have to explicitly handle with a |
I don't see it that way. First step should be to implement typechecking of throw types the same way as return types. Then add throw types to standard library definitions which are part of TypeScript. Then the library authors could just regenerate their definitions like always and have the throw types inferred the same way return types are inferred when you don't specify them. For backward compatibility, functions without a throw type would be treated as throw any or throw unknown, so if a library depends on another library which haven't been updated yet, it just gets it's own throw types inferred as unknown. |
|
"What if TS had typed/checked exceptions" is off-topic here; this is not a place to re-enact #13219 |
"Talk about exceptions here" 🤔 ETA: any chance the TS team would consider enabling the GitHub "Discussions" feature for posts like these? Issues are terrible at capturing long-running discussions because once there are too many comments, context gets lost behind the "Load more..." link and search breaks down. |
|
I agree that the topic of this thread is unclear about what's already been litigated, but it has already been extensively litigated (even if I'm bummed about the result). I think this thread was intended to be more of "other options, now that that decision has been made" |
|
I only found this issue after the previous one was closed. My usecase was: I wanted to enforce that a handler function will throw only HttpErrors: type Handler<T, E extends HttpError<number>> = (req: Request) => T throw Eand I wanted to infer possible responses and their status codes based on what the function actually throws: type handlerResponse<H extends Function> =
H extends (...args: any[]) => infer T throw HttpError<infer N>
? TypedResponse<200, T> | TypedResponse<N, string>
: never |
|
I wonder if a simple util function could suffice. function attempt<E extends Error, T>(cb: () => T, ...errors: E[]): [T | null, E | null] {
let error: E | null = null;
let value: T | null = null;
try {
value = cb();
} catch (err) {
const matches = errors.find(errorClass => err instanceof errorClass);
if (matches) {
error = err;
} else {
throw err;
}
}
return [value, error];
}
class StringEmptyError extends Error {}
function getStringLength(arg: string) {
if (!arg.trim()) throw new StringEmptyError();
return arg.length;
}
// Usage:
const [length, error] = attempt(() => getStringLength(" "), StringEmptyError);
// if error is not a StringEmptyError, it is thrown
if (error) {
// error is a StringEmptyError
}This is just an idea. It would need to be improved to handle async functions. It could also probably be changed to compose new functions to avoid repeating callbacks and errors, for example: class StringEmptyError extends Error {}
class SomeOtherError extends Error {}
function getStringLength(arg: string) {
if (!arg.trim()) throw new StringEmptyError();
return arg.length;
}
// ... Assuming `throws` is defined
const getStringLength = throws(
(arg: string) => {
if (!arg.trim()) throw new StringEmptyError();
return arg.length;
},
StringEmptyError,
SomeOtherError
);
// Same usage, but a bit simpler
const [length, error] = getStringLength(" ");
// if error is not a StringEmptyError or SomeOtherError, it is thrown
if (error) {
// error is a StringEmptyError or SomeOtherError
} |
|
That's a handy wrapper for, uh, turning TS into Go I guess? (There are worse ideas out there!) But I can't figure out how this helps with static analysis to enforce error checking. In particular, it looks like |
|
My example wasn't meant to be perfect. It was just an idea on how to accomplish some way of better error handling. Example: class StringEmptyError extends Error {}
class SomeOtherError extends Error {}
const getStringLength = throws(
(arg: string) => {
if (!arg.trim()) throw new StringEmptyError();
return arg.length;
},
StringEmptyError,
SomeOtherError
);
const length = getStringLength(' ')
.catch(SomeOtherError, err => console.error(err))
.catch(StringEmptyError, err => console.error(err));
console.log(length); // would be undefined in this case, it hits StringEmptyErrorSee CodeSandbox for a working If you don't add Update: I also took the liberty of publishing this in a After some further development on this there are some obvious problems. But I think they can be addressed. UPDATE 2: I've added more improvements to |
This is really neat, btw I would suggest to not enforce try catch, because it's legit to ignore the error and let it propagate without putting eslint comments to disable the rule everywhere. Instead I would propose to force the user to annotate a function that is not catching an error with a @throws as well, this way the user can choose to ignore errors but at least the function openly declares that it may @throws. |
|
We can always use and wrap something like Rest style Result to handle error types, but long as actual throw error types are not part of TS this is just extra layer hack (same as trying to handle this on JSDoc) function hello(arg: unknown): string throws<TypeError> {}... and have defaults like |
|
I'd like to re-plug a library I put together, since it's more refined than the examples I posted before. It lets you wrap a given function with enforced error catching, using syntax with similar verbosity when compared to a function using
import { throws } from 'ts-throws';
class StringEmptyError extends Error {}
class NoAsdfError extends Error {}
const getStringLength = throws(
(str: string) => {
if (!str.trim()) throw new StringEmptyError();
if (str === 'asdf') throw new NoAsdfError();
return str.length;
},
{ StringEmptyError, NoAsdfError }
);
/*
`throws` will force you to catch the provided errors.
It dynamically generates catch* methods based on the object of errors
you provide. The error names will be automatically capitalized.
*/
let length = getStringLength(' ')
.catchStringEmptyError(err => console.error('String is empty'))
.catchNoAsdfError(err => console.error('String cannot be asdf'));
// length is undefined, logged 'String is empty'
length = getStringLength('asdf')
.catchStringEmptyError(err => console.error('String is empty'))
.catchNoAsdfError(err => console.error('String cannot be asdf'));
// length is undefined, logged 'String cannot be asdf'
length = getStringLength(' ')
.catchStringEmptyError(err => console.error('String is empty'))
// Only one error caught, `length` is:
// { catchNoAsdfError: (callback: (err: NoAsdfError) => void) => number | undefined }
// Function logic not invoked until last error is handled with `.catch`
length = getStringLength('hello world')
.catchStringEmptyError(err => console.error('String is empty'))
.catchNoAsdfError(err => console.error('String cannot be asdf'));
// length is 11One improvement might be error pattern matching for things like I think the only advantage that a native |
i would highly discourage leveraging on I would suggest returning the errors instead of throwing them. |
One of the benefits here is that it's plug-and-play with functions that already throw. You don't need to modify the function at all, just wrap it (this still introduces a breaking change for consumers, as would returning an error.) Your suggestion would require refactors whose cost outweighs the value of performance in most scenarios. If you are running first-party functions in a loop where error handling performance becomes significant, those functions shouldn't throw to begin with as you suggested. My solution does not aim to address those use-cases. My goal was to augment native error checking capabilities, making it more convenient/type-safe without needing to change the "throwing" code. Any overhead introduced would be negligible in 90% of use-cases, and in the latter 10% the library would not be applicable. That being said, I'll benchmark the library and see where optimizations can be made. Update: I'm actually glad this got brought up. I put together some benchmarks and discovered my implementation was a bit over-complicated. Originally, In this benchmark, these were the results: |
The additional overhead is coming from generating the call stack when new Error() is called. That should be avoided as much as possible in frequently invoked code. The way I see it So as a rule a would avoid to use the error class for expected situations, eg: the string in input is empty and it shouldn't be. |
This is interesting. By not extending |
|
+1 to throwing untyped exceptions. Knowing which function calls are supposed to throw anything is the 90-10 solution we're after. function foo(bar: string): boolean throws {
if (someConditionNotMet()) {
throw 'an error';
}
}Example error foo('bar');
// ^💥TSError: Throwable not caught or rethrown.Example handled casesCatchtry {
foo('bar');
} catch (err) {
// ...
}Rethrowfunction bar() throws {
foo('bar');
}Swift equivalent (for posterity)Or Swift style (if a keyword would be easier on the AST): function bar() throws {
try foo('bar');
}We get a little derailed when we start thinking too hard into the future about typed exceptions. A simpled, opt-in, mark and unmark system would be a perfect place to start. |
|
I don't know about you but when I have problems with an unexpected exception, it's never the kind of problem where just swallowing it and blundering forward would have solved anything. Do other people really need a shorthand for that? |
|
I won't go into the merits or flaws of a shorthand, I was just pulling an illustrative example from our Swift friends. +1 for "blundering forward", excellent phrase. edit: updated above example to remove the shorthand, so as not to distract/derail. |
@KashubaK I don't like this solution because it forces the developer to mix error handling and logic flow. With throws, it's possible to write the whole happy path of the function separate from the error handling, which I think it's much more easy to read. Compare these two pseudocodes and tell me which one is easier to follow: fn getIntValueFromFile(path) {
fileHandler = open(path)
contents = read(fileHandler)
parsedValue = parseInt(contents)
return parsedValue
} catch (UnableToOpenFile err) {
// handle open error
return NaN
} catch (NotAnInt err) {
// handle parseInt error
return NaN
}against fn getIntValueFromFile(path) {
fileHandler = open(path)
if (fileHandler instanceof UnableToOpenFile) {
// handle open error
return NaN
}
contents = read(fileHandler)
parsedValue = parseInt(contents)
if (parsedValue instanceof NotAnInt) {
// handle parseInt error
return NaN
}
return parsedValue
}The first one is my preferred way to do error handling in other languages (I've thrown in Exlixir's implicit try just because I thinks it's a great idea), and having the language itself help me with what errors to handle would be a great thing to have. |
Do you mean to tag me in this? I'm not proposing people handle returned errors as you compared against. However |
|
@KashubaK Yes, I did. I quoted a part of your answer, and this is the code you suggested: const result = wow('haha');
if (result instanceof Error) {
// Handle the error
return;
}I my second exemple I tried to show how it would look inside a function body and how, to me, it doesn't seem very ergonomic. (but maybe I got something wrong, as it's known to happen) |
Ohhhhh gotcha. In retrospect, I'm not a huge fan of that either. I was trying to look for "How can we achieve this in current TS" since the conversation of "How can TS change to support this new idea" isn't productive anymore. I wrote a library called |
|
I read all comments with my best effort before trying once more to beat down on a simingly dead horse but I may've missed if a similar post was already created so I appologize if that's the case. I appreciate the thorough analysis and feedback regarding the proposal to introduce ProblemIf TypeScript aims to be a statically-typed superset of JavaScript, it should support and enhance all features present in existing JavaScript. Exceptions and error handling, do not have direct static typing counterparts in TypeScript. Key Points:Exception Handling: TypeScript's current handling of exceptions using Ecosystem survey: It is true that many JavaScript libraries do not document exceptions explicitly. However, this should not deter TypeScript from offering a mechanism to improve this situation. By providing Language Capabilities and Cultural Absence: The absence of strongly-typed exceptions in JavaScript is largely due to historical and practical reasons rather than inherent language limitations. JavaScript’s evolution and its use in varied environments (e.g., browser, server) have contributed to this. However, TypeScript’s goal is to bring type safety to JavaScript, and incorporating Argument for Including throws ClausesTo address the concerns about the impracticality of immediate, widespread adoption and the issues with existing undocumented exceptions, I propose an incremental and flexible approach: Optional throws Clauses with Default Behavior: Functions can optionally specify function fetchData() throws NetworkError {
// Function logic
if (networkFails) {
throw new NetworkError("Failed to fetch data");
}
}Compiler Warnings and Documentation Encouragement: The TypeScript compiler can provide warnings for unhandled exceptions only if a function being called in its body explicitly declares a function processData() {
fetchData(); // Warning: unhandled NetworkError
}Enhanced Error Handling and Clarity: By encouraging developers to specify Aligning with TypeScript’s Goals: Introducing Reconsideration PointsAdoption and Documentation: As the TypeScript community adopts |
|
@callistino I like a lot of your points but honestly your ideas were pretty well covered in #13219. One key point is that the TS compiler does not generate "warnings", only errors -- any type issues are fatal. You wouldn't think this would be the end of the world, but it means that in a codebase with partial throws-clause coverage, the annotations are either useless (checking flag disabled) or deafening (checking flag enabled). Personally, I think that this is the first case I've seen of a language keyword that I would like to see added not for the benefit of the compiler, but for 3rd party tooling. The eslint folks have already said that marking promises as "never rejects" is too hard to implement without language support. As you point out, being able to mark unhandled, advertised / expected exceptions as a non-fatal linter warning would be great. And even Intellisense could maybe benefit from "hinting" about exception/rejection types. Still, I understand the team's hesitance to add a keyword that wouldn't actually deliver value to the type-checker. |
|
@callistino I'd encourage you to look at #57943, which tries to get the documentation and ergonomics benefits you're describing, without going for checked exceptions (or manually-maintained But it is trying to solve a similar problem, and in a way that I think makes sense to have in Typescript, rather than re-implementing in a linter, because it:
|
|
Most of what I'm reading from others about "type safe try/catch", it seems people want (Java's) function So as an example: // typescript
try {
myFunction()
} catch (e: Error) {
console.error(e)
}could generate into js: // javascript
try {
myFunction()
} catch (e) {
if (e instanceof Error) {
console.error(e)
} else {
throw e
}
}This would theoretically also allow us to catch multiple types with multiple catch statements, just like Java does, with something like // typescript
try {
myFunction()
} catch (e: Error) {
console.error(e)
} catch (e: OtherError) {
console.log("Other error occured")
}which would become // javascript
try {
myFunction()
} catch (e) {
if (e instanceof Error) {
console.error(e)
} else if (e instanceof OtherError) {
console.log("Other error occured")
} else {
throw e
}
}This would require changes to what is accepted as "valid" ts, compared to js, given that multiple catch-statements isn't valid js, but we already write invalid JS as TS, so it's not that far of a stretch. This would allow us to more easily catch specific errors using the existing try/catch system, without having to touch function syntax or anything of the sort, nor is the developer required to use monads for every function call. |
|
I think this has been suggested in the past, and shot down as a violation of "non goal" 5:
They really don't like emitting substantially-different JS, it should be possible to just strip away types and more or less generate a valid JS program. |
|
Ah, alright, that makes sense. And I guess it would be challenging to only be able to catch class-based types here. Something like this might be a challenge: type A = Error | string
type B = `${'Funny' | 'Boring'}Error`
try {}
catch (e: A) {}
catch (e: B) {}What would be generated for this, then? try {}
catch(e) {
if (e instanceof Error || typeof e === "string") {}
else if (typeof e === "string" && /^(Funny|Boring)Error$/.test(e)) { /** this test can get super-complex */ }
else { throw e }
}Fair fair. |
|
I'd like to add my two cents here as well. When working on the backend, I encounter two different types of errors:
Expected ErrorsExample: When handling the user registration form, we should return something like
Usually, such errors describe domain-specific cases. Some protocols working over HTTP typically use the Unexpected ErrorsExample: When handling the same user registration form, we should return a
Usually, such errors describe infrastructure failures or (explicit or implicit) assertion mismatches. These errors are often returned with an HTTP status of Approach for Expected ErrorsReturning to the user registration form... type UserRegistrationErrorCode =
| 'ERR_USER_IS_ALREADY_REGISTERED'
| // --- snip --
;
class UserRegistrationError extends Error {
constructor(public code: UserRegistrationErrorCode) {
super(`${code}`);
}
}
interface IUserService {
register(userData: UserRegistrationData): Promise<User | UserRegistrationError>;
}So, we simply return the error explicitly from the On the calling side, we should handle it like this: async function signUp(
userData: UserSignUpData,
): Promise<{ user: User, session: Session } | UserRegistrationError> {
const registrationData = prepareUserRegistrationData(userData);
const user = await userService.register(registrationData);
if (user instanceof Error) {
// user type is correctly narrowed to UserRegistrationError
return user;
}
const session = await sessionService.startUserSession(user);
return { user, session };
}In general, this approach looks simple and robust, but This method lacks the implicit error propagation that exceptions provide. To Approach for Unexpected ErrorsSimply throw them. Catch the corresponding exception in the error boundary Sometimes, we need to transform an unexpected error into an expected one. Therefore, the error boundary might not be the only place where we catch It Seems Like We Have Everything We NeedUnfortunately, returning errors introduces a lot of noisy To address this, we need an operator that allows us to return the received error, async function signUp(
userData: UserSignUpData,
): Promise<{ user: User, session: Session } | UserRegistrationError> {
const registrationData = prepareUserRegistrationData(userData);
const user = try? await userService.register(registrationData);
const session = await sessionService.startUserSession(user);
return { user, session };
}Additionally, we need an operator that throws any Error-result if we know that async function signUp(
userData: UserSignUpData,
): Promise<{ user: User, session: Session }> {
const registrationData = prepareUserRegistrationData(userData);
const user = try! await userService.register(registrationData);
const session = await sessionService.startUserSession(user);
return { user, session };
}I've chosen Additionally, instead of checking if the value is an instance of the type ResultValue<T, E> =
| { isError: false, value: T }
| { isError: true, value: E }
interface Result<T, E> {
[Symbol.result](): ResultValue<T, E>;
}So, the protocol for
For
SummaryI understand the TS Team's position regarding language extensions: prioritizing ECMAScript first. This approach makes sense, but the JS community is currently considering a proposal for a "Safe Assignment Operator". However, they are not particularly interested in strict error typing, so we cannot expect TC39 to address typed errors anytime soon. So, We need a Result Type in TypeScript to tackle the typed errors. |
|
About I've also tried the similar one Function.prototype.throw The code looks like the following; const sqrt = (a: number): number =>
a >= 0 ? Math.sqrt(a) : sqrt.throw(new SqrtError());
sqrt.throws = [SqrtError] as const;
const div = (a: number, b: number): number =>
b !== 0 ? a / b :
a !== 0 ? div.throw(new DivisionError("ERR_DIV_BY_ZERO")) :
div.throw(new DivisionError("ERR_INDETERMINATE_FORM"));
div.throws = [DivisionError] as const;
const quadraticEquation = (a: number, b: number, c: number): [number, number] => {
const rootOfDiscriminant = sqrt(b * b - 4 * a * c);
return [
div(-b - rootOfDiscriminant, 2 * a),
div(-b + rootOfDiscriminant, 2 * a),
];
}
quadraticEquation.throws = [
...div.throws,
...sqrt.throws,
] as const;The approach allows to reflect the "expected" errors in function type that could be using in interfaces as well, |
|
I haven't read literally every comment here (there are just too many), but I'm a bit confused about why there's so much focus on enforcing that certain exceptions get caught. In the original proposal for the Isn't this level of control sufficient for most cases? Why should TypeScript enforce that every time you call a function, you have to wrap it in a try/catch block? Such enforcement seems unnecessary when we can already manage exception types through |
Acknowledgement
Comment
#13219 is locked so that the conclusion doesn't get lost in the discussion, so talk about exceptions here instead
The text was updated successfully, but these errors were encountered: