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.

Sign up for GitHub

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

Jump to bottom

Better handling of >= 300 error messages for non-text responses #11

Open
wbobeirne opened this issue Aug 23, 2023 · 0 comments
Open

Better handling of >= 300 error messages for non-text responses #11

wbobeirne opened this issue Aug 23, 2023 · 0 comments
Labels
enhancement New feature or request

Comments

@wbobeirne
Copy link
Collaborator

js-lnurl/src/index.ts

Lines 68 to 70 in 9660f19

if (r.status >= 300) {
throw new Error(await r.text())
}

Throwing with an error message of just text can result in some really messy error messages if the website decides to return something like an HTML 404 page. For instance, Alby results in the following if you try a garbage name:

https://getalby.com/.well-known/lnurlp/gnkwalgnwaklan returned error: <!DOCTYPE html>
<html>
<head>
  <title>The page you were looking for doesn't exist (404)</title>
  <meta name="viewport" content="width=device-width,initial-scale=1">
  <link rel="icon" href="/alby_icon_yellow_128x128.png" />
  <style>
  .rails-default-error-page {
    background-color: #EFEFEF;
    color: #2E2F30;
    text-align: center;
    font-family: arial, sans-serif;
    margin: 0;
  }
  .rails-default-error-page div.dialog {
    width: 95%;
    max-width: 33em;
    margin: 4em auto 0;
  }
  .rails-default-error-page div.dialog > div {
    border: 1px solid #CCC;
    border-right-color: #999;
    border-left-color: #999;
    border-bottom-color: #BBB;
    border-top: #B00100 solid 4px;
    border-top-left-radius: 9px;
    border-top-right-radius: 9px;
    background-color: white;
    padding: 7px 12% 0;
    box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
  }
  .rails-default-error-page h1 {
    font-size: 100%;
    color: #730E15;
    line-height: 1.5em;
  }
  .rails-default-error-page div.dialog > p {
    margin: 0 0 1em;
    padding: 1em;
    background-color: #F7F7F7;
    border: 1px solid #CCC;
    border-right-color: #999;
    border-left-color: #999;
    border-bottom-color: #999;
    border-bottom-left-radius: 4px;
    border-bottom-right-radius: 4px;
    border-top-color: #DADADA;
    color: #666;
    box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
  }
  </style>
</head>
<body class="rails-default-error-page">
  <!-- This file lives in public/404.html -->
  <div class="dialog">
    <div>
      <h1>The page you were looking for doesn't exist.</h1>
      <p>You may have mistyped the address or the page may have moved.</p>
    </div>
    <p>If you are the application owner check the logs for more information.</p>
  </div>
</body>
</html>

This can be bad if your app wants to show the error messages in a user facing way, and can blow up your logs with some really large payloads if a website had a really detailed 404 page.

IMO the library should instead use the statusText property of the response to get a short error if it was an unexpected response status. Perhaps if a developer does want the full response logged for debugging, some kind of debug env var or flag can be sent?
@wbobeirne wbobeirne added the enhancement New feature or request label Aug 23, 2023
@wbobeirne wbobeirne changed the title Better handling of >= 400 error messages for non-text responses Better handling of >= 300 error messages for non-text responses Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@wbobeirne