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

Starting OpenMCT fresh causes persistent error message #5914

Closed
2 of 7 tasks
ozyx opened this issue Oct 26, 2022 · 10 comments · Fixed by #5965
Closed
2 of 7 tasks

Starting OpenMCT fresh causes persistent error message #5914

ozyx opened this issue Oct 26, 2022 · 10 comments · Fixed by #5965

Comments

@ozyx
Copy link
Contributor

ozyx commented Oct 26, 2022

Summary

When starting OpenMCT with a fresh instance (fresh DB, either local or CouchDB), a persistent error message is generated:

image

The persistence of the error message and the red color would serve to indicate that something has gone terribly wrong. And in some cases, this may be true. But in the case of a fresh DB, this is to be expected. Since the mine folder ('My Items', or otherwise named) does not yet exist, this will trigger an attempt to create the mine folder.

The existence of this error message impacts our visual tests as it now shows up in every snapshot we take (since the message needs to be manually dismissed).

Expected vs Current Behavior

We should handle this specific case gracefully somehow, and not generate an error unless the attempt to create the mine folder fails (after noticing it does not yet exist).

Steps to Reproduce

  1. Clear browser cache
  2. Start openMCT fresh
  3. See a persistent error message "Failed to retrieve object mine"
  4. Notice that error message does not go away unless manually dismissed

Environment

  • Open MCT Version: 2.1.2
  • Deployment Type: local
  • OS: MacOS
  • Browser: Chrome

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information

@unlikelyzero
Copy link
Collaborator

Adding blocker as this is blocking the utility of the visual tests

@akhenry
Copy link
Contributor

akhenry commented Oct 27, 2022

This is almost certainly due to #5488 which fixed an issue where persistence errors were being suppressed inappropriately. We clearly need to suppress some errors, but as high up the chain as possible so we are not losing important user feedback. Perhaps our object interceptor API could be modified to allow interceptors to suppress errors under certain circumstances.

@akhenry
Copy link
Contributor

akhenry commented Oct 27, 2022

@ozyx does this have a functional impact? And is there a workaround?

@ozyx
Copy link
Contributor Author

ozyx commented Oct 27, 2022

@ozyx does this have a functional impact? And is there a workaround?

There is no functional impact as far as I'm aware. It's mainly impacting our visual and couchDB tests

@akhenry
Copy link
Contributor

akhenry commented Oct 27, 2022

Actually, I think this is just an object interceptor order issue in this case. The missingObjectInterceptor is kicking in before the myItemsInterceptor. Maybe we just need to reverse that order.

@akhenry
Copy link
Contributor

akhenry commented Oct 27, 2022

Reprioritizing as critical. It has no functional impact and trying to fix this to unblock release will introduce risk.

@ozyx
Copy link
Contributor Author

ozyx commented Oct 27, 2022

Actually, I think this is just an object interceptor order issue in this case. The missingObjectInterceptor is kicking in before the myItemsInterceptor. Maybe we just need to reverse that order.

Potential solution to implement prioritization for interceptors?:

//TODO: sort by priority

@akhenry akhenry added this to the 2.1.3 milestone Oct 29, 2022
@ozyx ozyx self-assigned this Nov 4, 2022
@ozyx
Copy link
Contributor Author

ozyx commented Nov 9, 2022

Splitting the CouchDB 404 off of this ticket onto its own, since I think it should be handled on the e2e side.

#5964

@ozyx ozyx changed the title Starting OpenMCT fresh causes persistent error message (and a 404 if using CouchDB) Starting OpenMCT fresh causes persistent error message Nov 9, 2022
@ozyx
Copy link
Contributor Author

ozyx commented Nov 9, 2022

Testing Notes

  1. Clear localStorage in the browser completely
  2. Start Open MCT with Local Storage plugin enabled
  3. Navigate to Open MCT
  4. Verify there is no error notification produced
  5. Verify that the "My Items" folder has been created by inspecting localStorage

Repeat the same steps above with CouchDB plugin and a fresh CouchDB database and verify that the
My Items" folder is created and no error notification is generated

scottbell added a commit that referenced this issue Nov 10, 2022
… first (#5965)

* feat: sort interceptors by priority

* fix(#5914): high priority for MyItemsInterceptor

* fix: create myItems if object is falsy

* test(e2e): update snapshots

Co-authored-by: Scott Bell <[email protected]>
@unlikelyzero unlikelyzero assigned michaelrogers and unassigned ozyx Nov 12, 2022
@michaelrogers
Copy link
Contributor

Fix verified during testathon on 10/14/2022.

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

Successfully merging a pull request may close this issue.

5 participants