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

fix: HTTP recovery in the same tab #876

Merged
merged 4 commits into from
May 13, 2020
Merged

fix: HTTP recovery in the same tab #876

merged 4 commits into from
May 13, 2020

Conversation

abueide
Copy link
Contributor

@abueide abueide commented May 6, 2020

Fixes #873

The changes caused some tests to fail, and I'm not sure my javascript skills are good enough to figure out how to fix them properly without just deleting them.

  419 passing (2s)
  1 pending
  7 failing

  1) requestHandler.onCompleted:
       with recoverFailedHttpRequests=true
         should redirect to default subdomain gateway on broken subdomain gateway request:
     AssertionError: tabs.create should be called with default subdomain gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:57:14)

  2) requestHandler.onCompleted:
       with recoverFailedHttpRequests=true
         should recover from unreachable third party public gateway by reopening on the public gateway:
     AssertionError: tabs.create should be called with IPFS default public gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:92:14)

  3) requestHandler.onErrorOccurred:
       with recoverFailedHttpRequests=true
         should redirect to default subdomain gateway on failed subdomain gateway request:
     AssertionError: tabs.create should be called with default subdomain gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:156:14)

  4) requestHandler.onErrorOccurred:
       with recoverFailedHttpRequests=true
         should recover from unreachable third party public gateway by reopening on the public gateway:
     AssertionError: tabs.create should be called with IPFS default public gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:182:14)

  5) requestHandler.onErrorOccurred:
       with recoverFailedHttpRequests=true
         should recover from unreachable HTTP server by reopening DNSLink on the active gateway:
     AssertionError: tabs.create should be called with DNSLink on local gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:191:14)

  6) requestHandler.onErrorOccurred:
       with recoverFailedHttpRequests=true
         should recover from failed DNS for .eth opening it on EthDNS gateway at .eth.link:
     AssertionError: tabs.create should be called with ENS resource on local gateway URL: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-gateway-recover.test.js:202:14)

  7) modifyRequest processing
       a failed main_frame request to /ipns/ipfs.io/blog
         should be updated to /ipns/blog.ipfs.io:
     AssertionError: expected false to be truthy
      at Context.<anonymous> (test/functional/lib/ipfs-request-workarounds.test.js:194:14)



----------------------------------------|----------|----------|----------|----------|-------------------|
File                                    |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------------------------------|----------|----------|----------|----------|-------------------|
All files                               |    61.29 |    57.17 |       59 |    62.74 |                   |
 lib                                    |    59.53 |    55.52 |    56.94 |    61.06 |                   |
  context-menus.js                      |    58.24 |     7.69 |    42.86 |    60.47 |... 71,173,174,177 |
  copier.js                             |     9.38 |        0 |    14.29 |     9.38 |... 59,61,68,69,70 |
  dir-view.js                           |      100 |       50 |      100 |      100 |             10,33 |
  dnslink.js                            |    68.81 |    66.67 |    81.25 |    73.27 |... 56,159,161,167 |
  http-proxy.js                         |    25.81 |     6.25 |    33.33 |    24.14 |... 8,92,96,97,101 |
  inspector.js                          |       50 |      100 |       50 |       50 |    10,11,12,13,14 |
  ipfs-companion.js                     |    27.88 |     8.05 |    15.38 |    29.38 |... 55,759,776,777 |
  ipfs-import.js                        |    21.43 |        0 |    22.22 |    21.31 |... 09,110,111,112 |
  ipfs-path.js                          |     92.5 |    80.67 |      100 |    94.41 |... 25,250,310,359 |
  ipfs-protocol.js                      |    86.67 |     62.5 |      100 |    86.21 |       20,21,37,49 |
  ipfs-request.js                       |    87.88 |    84.41 |     93.1 |    90.95 |... 15,416,420,422 |
  mime-sniff.js                         |       80 |       60 |      100 |    92.31 |                33 |
  notifier.js                           |    27.27 |        0 |    33.33 |    27.27 |... 12,14,15,20,22 |
  on-installed.js                       |    55.56 |        0 |       50 |    55.56 |         8,9,17,18 |
  options.js                            |    91.43 |    77.55 |      100 |    91.18 |... 63,172,173,182 |
  precache.js                           |       30 |     8.33 |    30.77 |    32.14 |... 03,104,105,109 |
  runtime-checks.js                     |    93.33 |    96.43 |      100 |    93.33 |                16 |
  state.js                              |    94.74 |     62.5 |       80 |    94.29 |             50,53 |
 lib/ipfs-client                        |     61.7 |    52.63 |    43.75 |    63.04 |                   |
  embedded.js                           |    29.03 |        0 |        0 |       30 |... 37,44,45,47,48 |
  external.js                           |      100 |      100 |      100 |      100 |                   |
  index.js                              |       72 |    55.56 |    71.43 |    73.47 |... 62,64,65,66,89 |
 lib/ipfs-client/embedded-chromesockets |    29.61 |        0 |        0 |    30.41 |                   |
  config.js                             |    20.75 |        0 |        0 |    21.57 |... 56,159,161,162 |
  index.js                              |    21.57 |        0 |        0 |       22 |... 85,86,87,89,92 |
  libp2p-bundle.js                      |    34.48 |        0 |        0 |    34.48 |... 01,102,104,106 |
  libp2p.js                             |    68.42 |      100 |        0 |    72.22 |    24,25,26,32,87 |
 lib/ipfs-proxy                         |     76.7 |    74.85 |    74.12 |    78.48 |                   |
  access-control.js                     |    98.08 |    96.08 |      100 |      100 |            62,173 |
  enable-command.js                     |      100 |    90.91 |      100 |      100 |             24,48 |
  index.js                              |    43.18 |        0 |    15.38 |     47.5 |... 57,58,61,64,68 |
  pre-acl.js                            |       96 |    88.89 |      100 |      100 |             27,49 |
  pre-command.js                        |     91.3 |       75 |      100 |    95.24 |                21 |
  pre-mfs-scope.js                      |    97.01 |    92.59 |      100 |    96.72 |            72,121 |
  request-access.js                     |    12.28 |        0 |     8.33 |    13.73 |... 06,108,111,113 |
 pages/proxy-access-dialog              |    71.43 |       50 |       50 |    90.91 |                   |
  page.js                               |    71.43 |       50 |       50 |    90.91 |                54 |
 pages/proxy-acl                        |    93.48 |       90 |    88.89 |      100 |                   |
  page.js                               |     87.5 |    93.33 |    81.82 |      100 |                38 |
  store.js                              |      100 |       80 |      100 |      100 |                 3 |
 popup                                  |      100 |    57.14 |      100 |      100 |                   |
  logo.js                               |      100 |    57.14 |      100 |      100 |          6,7,8,13 |
----------------------------------------|----------|----------|----------|----------|-------------------|

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @abueide, looks good! ❤️

The tests should be easy to fix actually: just replace every occurrence of tabs.create with tabs.update in test/functional/lib/ipfs-request-gateway-recover.test.js – we are just checking if WebExtension API was called, and the only thing changed is the name of the API methog :)

If that does not help, let me know, I'll take a closer look 👍

add-on/src/lib/ipfs-request.js Outdated Show resolved Hide resolved
@lidel lidel changed the title Fixes #873 fix: update tab instead of creating a new one during HTTP recovery May 6, 2020
@abueide
Copy link
Contributor Author

abueide commented May 6, 2020

@lidel To check changes on tests I just do npm run dev-build and then npm test right? I'm still getting the same test failures after I changed them all.

@lidel
Copy link
Member

lidel commented May 8, 2020

@abueide ah, I believe you need to remove all occurrences of , openerTabId: 20 in ipfs-request-gateway-recover.test.js (tabs.update is called without it).

That should fix remaining tests :)

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All green, works as expected, I'm gonna merge this.

Thank you @abueide! 👍

@lidel lidel changed the title fix: update tab instead of creating a new one during HTTP recovery fix: HTTP recovery in the same tab May 13, 2020
@lidel lidel merged commit 0ce7974 into ipfs:master May 13, 2020
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.

Opening .eth creates 2 tabs
2 participants