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

metadata errors are NOT errors, prettify display #811

Merged
merged 3 commits into from
Apr 28, 2023

Conversation

taylordowns2000
Copy link
Member

@taylordowns2000 taylordowns2000 commented Apr 28, 2023

Closes #812

Notes for the reviewer

@josephjclark , the <Empty /> block doesn't seem to get returned, even when trying to use an adaptor that has no magic.

See my comments below. I was about to put the <Empty /> return after 'no_metadata_result' but realized that this is probably meant to mean something different, so I thought I'd ask.

  if (!metadata) {
    // I never seem to get reached...
    return <Empty adaptor={adaptor} />;
  }
  if (metadata === true) {
    return <div className="block m-2">Loading metadata...</div>;
  }
  if (metadata.error) {
    const { error } = metadata;
    let message = `An error occurred while loading metadata: ${error}`;

    if (error === 'no_credential')
      message =
        "Metadata can only be loaded once you've added a valid credential.";

    if (error === 'no_metadata_result')
      // When I use an adaptor that doesn't have magic, I get this error...
      message = `The ${adaptor} adaptor isn't returning any metadata.`;

    return <div className="block m-2">{message}</div>;
  }

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have verified that all appropriate authorization policies have been implemented and tested
  • If needed, I have updated the changelog
  • Taylor has QA'd this feature

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.05 ⚠️

Comparison is base (79492a1) 92.68% compared to head (92a1085) 92.64%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #811      +/-   ##
==========================================
- Coverage   92.68%   92.64%   -0.05%     
==========================================
  Files         163      163              
  Lines        3911     3916       +5     
==========================================
+ Hits         3625     3628       +3     
- Misses        286      288       +2     
Impacted Files Coverage Δ
lib/lightning/cli.ex 100.00% <ø> (ø)
lib/lightning/metadata_service.ex 62.50% <66.66%> (-0.47%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@taylordowns2000 taylordowns2000 marked this pull request as ready for review April 28, 2023 17:25
@taylordowns2000 taylordowns2000 removed the request for review from josephjclark April 28, 2023 17:26
@taylordowns2000 taylordowns2000 merged commit 5ac32fa into main Apr 28, 2023
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.

More helpful magic function errors
1 participant