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

Show error message upon import exception #647

Merged
merged 4 commits into from
Mar 13, 2021

Conversation

christianlupus
Copy link
Collaborator

This solves partially the issue described in #626 as it avoids an empty alert box.

@codecov
Copy link

codecov bot commented Mar 6, 2021

Codecov Report

Merging #647 (b947d6a) into master (c2ccb3c) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #647   +/-   ##
========================================
  Coverage      0.99%   0.99%           
  Complexity      452     452           
========================================
  Files            15      15           
  Lines          1414    1414           
========================================
  Hits             14      14           
  Misses         1400    1400           
Flag Coverage Δ Complexity Δ
integration 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
unittests 0.99% <0.00%> (ø) 0.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
lib/Controller/MainController.php 0.00% <0.00%> (ø) 40.00 <0.00> (ø)

@christianlupus
Copy link
Collaborator Author

christianlupus commented Mar 6, 2021

@seyfeb as the original line in the AppNavi.vue came from the Axios update, I just wanted to make sure you are ok with this modification. Or should we check that at least some result was returned before we try to use it?

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 7, 2021

@seyfeb as the original line in the AppNavi.vue came from the Axios update, I just wanted to make sure you are ok with this modification. Or should we check that at least some result was returned before we try to use it?

I think there is always some response from the server?* To be sure one could provide a default feedback to the user if it’s empty.

  • I’m not sure what the response contains if there is a timeout

Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
@christianlupus christianlupus force-pushed the fix/626-print-import-error-message branch from ce741bd to be3e340 Compare March 13, 2021 17:36
Coded along https://github.com/axios/axios#handling-errors

Signed-off-by: Christian Wolf <github@christianwolf.email>
@christianlupus
Copy link
Collaborator Author

@seyfeb I had a look at the axios documentation once more and came up with a bit more secure code in the sense that exceptions are handled more explicitly.

@christianlupus christianlupus requested a review from seyfeb March 13, 2021 17:50
@seyfeb
Copy link
Collaborator

seyfeb commented Mar 13, 2021

I think this should be fine, except for the failing linters 😄

Signed-off-by: Christian Wolf <github@christianwolf.email>
@christianlupus
Copy link
Collaborator Author

The linters are taken care of.
Good to have them in the CI chain 😄

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

Successfully merging this pull request may close these issues.

2 participants