Merge pull request #9866 from Turbo87/readme-loading

Show "Retry" button for README loading errors

Author: Turbo87

app/controllers/crate/version.js

@@ -43,7 +43,7 @@ export default class CrateVersionController extends Controller {
       : await version.loadReadmeTask.perform();
 
     // If the README contains `language-mermaid` we ensure that the `mermaid` library has loaded before we continue
-    if (readme.includes('language-mermaid') && !this.mermaid.loadTask.lastSuccessful?.value) {
+    if (readme && readme.includes('language-mermaid') && !this.mermaid.loadTask.lastSuccessful?.value) {
       try {
         await this.mermaid.loadTask.perform();
       } catch (error) {

app/models/version.js

@@ -122,6 +122,10 @@ export default class Version extends Model {
   loadReadmeTask = keepLatestTask(async () => {
     if (this.readme_path) {
       let response = await fetch(this.readme_path);
+      if (response.status === 404 || response.status === 403) {
+        return;
+      }
+
       if (!response.ok) {
         throw new Error(`README request for ${this.crateName} v${this.num} failed`);
       }

app/styles/crate/version.module.css

@@ -25,7 +25,7 @@
     }
 }
 
-.no-readme {
+.no-readme, .readme-error {
     padding: var(--space-l) var(--space-s);
     text-align: center;
     font-size: 20px;
@@ -39,6 +39,13 @@
     }
 }
 
+.retry-button {
+    composes: yellow-button from '../shared/buttons.module.css';
+    display: block;
+    text-align: center;
+    margin: var(--space-s) auto 0;
+}
+
 .placeholder-title {
     width: 30%;
     height: 25px;

app/templates/crate/version.hbs

@@ -25,6 +25,19 @@
       <article aria-label="Readme" data-test-readme>
         <RenderedHtml @html={{this.readme}} local-class="readme" />
       </article>
+    {{else if this.loadReadmeTask.last.error}}
+      <div local-class="readme-error" data-test-readme-error>
+        Failed to load <code>README</code> file for {{this.crate.name}} v{{this.currentVersion.num}}
+
+        <button
+          type="button"
+          local-class="retry-button"
+          data-test-retry-button
+          {{on "click" (perform this.loadReadmeTask)}}
+        >
+          Retry
+        </button>
+      </div>
     {{else}}
       <div local-class="no-readme" data-test-no-readme>
         {{this.crate.name}} v{{this.currentVersion.num}} appears to have no <code>README.md</code> file

e2e/acceptance/readme-rendering.spec.ts

@@ -1,4 +1,5 @@
-import { test, expect } from '@/e2e/helper';
+import { expect, test } from '@/e2e/helper';
+import { Response } from 'miragejs';
 
 const README_HTML = `
 <p><strong>Serde is a framework for <em>ser</em>ializing and <em>de</em>serializing Rust data structures efficiently and generically.</strong></p>
@@ -112,4 +113,29 @@ test.describe('Acceptance | README rendering', { tag: '@acceptance' }, () => {
     await page.goto('/crates/serde');
     await expect(page.locator('[data-test-no-readme]')).toBeVisible();
   });
+
+  test('it shows an error message and retry button if loading fails', async ({ page, mirage }) => {
+    await page.exposeBinding('resp200', () => new Response(200, { 'Content-Type': 'text/html' }, 'foo'));
+
+    await mirage.addHook(server => {
+      let crate = server.create('crate', { name: 'serde' });
+      server.create('version', { crate, num: '1.0.0' });
+
+      server.logging = true;
+      // Simulate a server error when fetching the README
+      server.get('/api/v1/crates/:name/:version/readme', {}, 500);
+    });
+
+    await page.goto('/crates/serde');
+    await expect(page.locator('[data-test-readme-error]')).toBeVisible();
+    await expect(page.locator('[data-test-retry-button]')).toBeVisible();
+
+    await page.evaluate(() => {
+      // Simulate a successful response when fetching the README
+      server.get('/api/v1/crates/:name/:version/readme', {});
+    });
+
+    await page.click('[data-test-retry-button]');
+    await expect(page.locator('[data-test-readme]')).toHaveText('{}');
+  });
 });

mirage/route-handlers/crates.js

@@ -374,12 +374,12 @@ export function register(server) {
     const { name, version: versionNum } = request.params;
     const crate = schema.crates.findBy({ name });
     if (!crate) {
-      return new Response(404, { 'Content-Type': 'text/html' }, '');
+      return new Response(403, { 'Content-Type': 'text/html' }, '');
     }
 
     const version = schema.versions.findBy({ crateId: crate.id, num: versionNum });
     if (!version || !version.readme) {
-      return new Response(404, { 'Content-Type': 'text/html' }, '');
+      return new Response(403, { 'Content-Type': 'text/html' }, '');
     }
 
     return new Response(200, { 'Content-Type': 'text/html' }, version.readme);

tests/acceptance/readme-rendering-test.js

@@ -1,6 +1,8 @@
+import { click } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 
 import percySnapshot from '@percy/ember';
+import { Response } from 'miragejs';
 
 import { setupApplicationTest } from 'crates-io/tests/helpers';
 
@@ -112,4 +114,25 @@ module('Acceptance | README rendering', function (hooks) {
     await visit('/crates/serde');
     assert.dom('[data-test-no-readme]').exists();
   });
+
+  test('it shows an error message and retry button if loading fails', async function (assert) {
+    let crate = this.server.create('crate', { name: 'serde' });
+    this.server.create('version', { crate, num: '1.0.0' });
+
+    // Simulate a server error when fetching the README
+    this.server.get('/api/v1/crates/:name/:version/readme', {}, 500);
+
+    await visit('/crates/serde');
+    assert.dom('[data-test-readme-error]').exists();
+    assert.dom('[data-test-retry-button]').exists();
+
+    // Simulate a successful response when fetching the README
+    this.server.get(
+      '/api/v1/crates/:name/:version/readme',
+      () => new Response(200, { 'Content-Type': 'text/html' }, 'foo'),
+    );
+
+    await click('[data-test-retry-button]');
+    assert.dom('[data-test-readme]').hasText('foo');
+  });
 });

tests/mirage/crates/versions/readme-test.js

@@ -11,15 +11,15 @@ module('Mirage | GET /api/v1/crates/:id/:version/readme', function (hooks) {
 
   test('returns 404 for unknown crates', async function (assert) {
     let response = await fetch('/api/v1/crates/foo/1.0.0/readme');
-    assert.strictEqual(response.status, 404);
+    assert.strictEqual(response.status, 403);
     assert.strictEqual(await response.text(), '');
   });
 
   test('returns 404 for unknown versions', async function (assert) {
     this.server.create('crate', { name: 'rand' });
 
     let response = await fetch('/api/v1/crates/rand/1.0.0/readme');
-    assert.strictEqual(response.status, 404);
+    assert.strictEqual(response.status, 403);
     assert.strictEqual(await response.text(), '');
   });
 
@@ -28,7 +28,7 @@ module('Mirage | GET /api/v1/crates/:id/:version/readme', function (hooks) {
     this.server.create('version', { crate, num: '1.0.0' });
 
     let response = await fetch('/api/v1/crates/rand/1.0.0/readme');
-    assert.strictEqual(response.status, 404);
+    assert.strictEqual(response.status, 403);
     assert.strictEqual(await response.text(), '');
   });