Skip to content
This repository has been archived by the owner on Aug 28, 2020. It is now read-only.

SettingsError Event #708

Closed
Skillz4Killz opened this issue May 30, 2019 · 6 comments
Closed

SettingsError Event #708

Skillz4Killz opened this issue May 30, 2019 · 6 comments

Comments

@Skillz4Killz
Copy link
Contributor

Skillz4Killz commented May 30, 2019

Describe your proposal

It would be really nice to have a event that is emitted whenever there is some error in Settings.

Use-Cases for your proposal

This is the current way of handling errors. Duplicate this over 100s of files on larger bots and you get a lot of extra lines that just are not needed if there was a nice event handler for these errors.

const { errors } = await settings.update(key, value, options)
if (errors.length) this.client.emit(`error`, errors.join(`\n`))

Current Workaround: Using the functions plugin

await this.client.helpers.update(key, value, options)

Expected and actual behavior

It would be nice to do something like below and any and all errors are emitted in the settingsError event.

await settings.update(key, value)

Further details

N/A

@kyranet
Copy link
Contributor

kyranet commented May 30, 2019

I disagree to this, but I see an usecase for a settingsError event: when the sync fails, which is currently not handled correctly, and could use some easy management:

if (this._synced && this.schema.size) settings.sync(true).catch(err => this.client.emit('error', err));

@Skillz4Killz
Copy link
Contributor Author

Skillz4Killz commented May 30, 2019

const { errors } = await guild.settings.update(key, value)
if (errors.length) this.client.emit('error', errors.join('\n'))

const updatedUser = await user.settings.update(key, value)
if (updatedUser.errors.length) this.client.emit(`error`, updatedUser.errors.join(`\n`))

return null

VS:

await guild.settings.update(key, value)
return user.settings.update(key, value)

7 lines down to 2 lines of code. Individually handling emitting errors is kinda bad when u have a larger bot. If the settingsError event is for another use case, it would also be possible to do it other ways.

await guild.settings.update(key, value, { emitErrors: true })
return user.settings.update(key, value, { emitErrors: true })

Then somewhere in the update method you can do

if (options.emitErrors && errors.length) client.emit.......

@kyranet
Copy link
Contributor

kyranet commented May 30, 2019

Alternative, using what's already available in the Settings branch:

await guild.settings.update(key, value, { throwOnError: true });
await user.settings.update(key, value, { throwOnError: true });

@tech6hutch
Copy link
Contributor

Other possibilities:

  • You could make a function that handles the errors, if any, and pass it to Promise#then: await guild.settings.update(key, value).then(handleErrors)

  • You could make an extendable on SettingsFolder#update

@kyranet
Copy link
Contributor

kyranet commented Jul 20, 2019

@Skillz4Killz is the current (and next with #752) interface enough satisfactory to close this issue?

@Skillz4Killz
Copy link
Contributor Author

@kyranet I believe throwOnError solved all my needs already.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants