enso-ui / select

Select
MIT License
2 stars 3 forks source link

Error handling on SelectField/VueSelect #17

Closed robbykrlos closed 3 years ago

robbykrlos commented 3 years ago

This is a bug.

Prerequisites

Description

Server error (500) does not show up in UI is errors are coming from Options.

Steps to Reproduce

  1. Open an option controller.
  2. Break the option query (Ex: SELECTTT *.....)
  3. Open a form that contains that options select.
  4. Initial load, or even clicking and triggering the option select to load will not give any negative feedback in regards to the error.

Expected behavior

Toastr with "Oups, something went wrong"

Actual behavior

No visible error (for normal users), just a console 500 error.

We tracked down a possible issue, and it might "the fix" but we want you also to validate this. In case you agree, we can do a PR:

@enso-ui/forms/src/bulma/fields/SelectField.vue is using the vue-select component not the enso-select component. and vue-select component does not set error-handler:

@enso-ui/select/src/bulma/EnsoSelect.vue - OK

<template>
    <vue-select :i18n="i18n"
        :error-handler="errorHandler"

...
 inject: ['errorHandler', 'i18n', 'route'],

@enso-ui/select/src/bulma/VueSelect.vue - NOT OK - missing :error-handler="errorHandler" and inject...

<core-select v-bind="$attrs"
        v-on="$listeners"
        ref="select">

or

set the error-handler on @enso-ui/forms/src/bulma/fields/SelectField.vue ?

<template>
    <vue-select v-model="field.value"
        :error-handler="errorHandler"
        :has-error="errors.has(field.name)"
        :i18n="i18n"
aocneanu commented 3 years ago

@gandesc what do you think about this? Might be a good idea to add in the global error handler a toaster for 500 errors.

gandesc commented 3 years ago

Hi @robbykrlos

I'm seeing the error handler set in @enso-ui/forms/src/bulma/fields/SelectField.vue link

Also, as expected, upon breaking the Options controller, I'm getting the toastr notification.

I'm on @enso-ui/forms 2.3.18

What are we missing here?

aocneanu commented 3 years ago

maybe he's in production and does not use sentry. in this case the toastr is not shown.

robbykrlos commented 3 years ago

maybe he's in production and does not use sentry. in this case the toastr is not shown.

Yes, this is correct, this came from a QA system that is set to PROD + we do not use sentry (not even on local)

robbykrlos commented 3 years ago

Hi @robbykrlos

I'm seeing the error handler set in @enso-ui/forms/src/bulma/fields/SelectField.vue link

Also, as expected, upon breaking the Options controller, I'm getting the toastr notification.

I'm on @enso-ui/forms 2.3.18

What are we missing here?

This is also correct - we are not using this @enso-ui/forms 2.3.18

  "name": "@enso-ui/forms",
  "version": "2.3.9",

I thought I've checked the latest version on github, but only checked the vue-select, not the selectField component.

I'll consider this fixed by 4.8.0 (@enso-ui/forms 2.3.18) then. As mentioned in the ticket description we are still on 4.7.1. Will upgrade soon.

I consider this closed, unless you still see something to be done for "prod + sentry off". Feel free to close this.

Thanks!