URL completa no arquivo ERB de assets --\u003e problemas com multisite

A nova implementação do service worker do workbox usa UrlHelper.absolute. Como se trata de um ativo compilado, ele armazena a URL completa, contendo o nome de host completo do host principal em um ambiente multissítio.

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/service-worker.js.erb#L3-L6

Acho que deveria usar UrlHelper.local_cdn_url em vez disso. Isso também elimina a necessidade de recompilar os ativos após alterar o nome de host.

1 curtida

Acho que o consenso geral é que os Service Workers nunca devem ser servidos por uma CDN:

Ah, então você está se referindo aos arquivos importScripts? Você tem um cluster multi-site onde cada site possui uma URL de CDN diferente?

1 curtida

Sim, importScripts e modulePathPrefix, nas linhas 3 e 6 desse arquivo ERB.

Mas você não me entendeu completamente. Os problemas estão ocorrendo onde não há CDN em um cluster multisite.

Tanto UrlHelper.absolute quanto UrlHelper.local_cdn_url podem lidar com situações com e sem CDN.

absolute:

sem cdn: https://primarysite.ofmultisitecluster.com/javascripts/workbox/workbox-sw.js **ruim - origem remota para todos os sites exceto o primário, expondo o hostname do cluster primário **
com cdn: //cdnurl/javascripts/workbox/workbox-sw.js

local_cdn_url:

sem cdn: /javascripts/workbox/workbox-sw.js bom - URL relativa
com cdn: //cdnurl/javascripts/workbox/workbox-sw.js

portanto, é o último que queremos.

O problema é que essas linhas (3 e 6) não se referem ao arquivo do service worker.

O service worker vem do domínio base (pois não pode ser servido por um CDN), mas ele importa scripts sob demanda em tempo de execução (neste caso, os arquivos da biblioteca workbox), e esses podem ser servidos por um CDN.

Portanto, o problema é que seu cluster multisite não tem um CDN configurado, o que expõe esse bug. O problema fica mascarado quando DISCOURSE_CDN está definido. Só queria saber por que isso não nos afeta.

1 curtida

Você está correto, editei minha segunda postagem para corrigir meus erros.
Não se trata do arquivo do service worker, ele está DENTRO do arquivo do service worker.

Sim, é exatamente nesse momento que o bug está sendo exposto.

3 curtidas

Só confirmando: meu diagnóstico está correto e isso é um bug que será corrigido? Há algo que precisem de nós que ajude?

3 curtidas

Sim, parece um bug que precisa ser corrigido. Vou resolver isso esta semana!

3 curtidas

Hmmm, acabei de testar no console do Meta:

## Atual
[1] pry(main)> UrlHelper.absolute("/javascripts/workbox/workbox-sw.js")
=> "https://d3bpeqsaub0i6y.cloudfront.net/javascripts/workbox/workbox-sw.js"

### Mudança proposta
[2] pry(main)> UrlHelper.local_cdn_url("/javascripts/workbox/workbox-sw.js")
=> "/javascripts/workbox/workbox-sw.js"

Para mim, essas não parecem ser funções intercambiáveis.

3 curtidas

Você está correto, local_cdn_url substitui uma URL local por uma URL de CDN. E isso nem mesmo é uma URL local, mas sim uma relativa.

Então, acho que isso seria suficiente no lugar dessas chamadas UrlHelper?

importScripts("<%= (Discourse.asset_host || '') + '/javascripts/workbox/workbox-sw.js' %>");

e

modulePathPrefix: (Discourse.asset_host || '') + '/javascripts/workbox',

1 curtida

Lendo a implementação atual de UrlHelper.absolute:

Parece que ela compõe a URL concatenando Discourse.base_url_no_prefix com o parâmetro quando o CDN é nil, que é o seu caso.

Então, o problema é que Discourse.base_url_no_prefix está sempre retornando o primeiro host no ambiente multisite?

Analisando o código :eyes:

o nome da variável aqui current_hostname na linha 288 fortemente sugere algo consciente de multisite :thinking:

e, por

parece que é. Ponto sem saída até agora…

Procurando em outro lugar, essa rota ganhou um tratamento especial porque os navegadores adoram acessá-la intensamente, e não podemos colocá-la em um CDN e deixar o problema para outra pessoa. Ao fazer isso, tivemos um bug envolvendo um vazamento em multisite, que foi corrigido por @sam há um ano:

Existe a possibilidade de que a maneira como você está servindo esse cluster multisite esteja armazenando em cache essa rota de forma vazada, como estávamos no início de 2018?

2 curtidas

Não, o problema é que isso ocorre durante a pré-compilação de assets, o que se torna um problema em ambientes multisite.
Portanto, a solução é não incluir o nome do host nos assets nunca (exceto para um CDN de assets, se ele estiver configurado, já que esse é sempre compartilhado entre os hosts multisite de qualquer forma).

1 curtida

@falco você viu minha solução proposta duas postagens acima desta?

Sim, mas nos meus testes, ele não cobre subpastas sem um CDN :sob:

Acho que precisaremos usar:

"#{Discourse.asset_host}#{Discourse.base_prefix}/javascripts/workbox"
1 curtida

Hmm, bom ponto sobre a subpasta.

Mas… Discourse.asset_host pode ser nil e nunca ouvi falar de Discourse.base_prefix?

Que tal isso:

importScripts("<%= (Discourse.asset_host || GlobalSetting.relative_url_root) + "/javascripts/workbox/workbox-sw.js" %>");

modulePathPrefix: "<%= (Discourse.asset_host || GlobalSetting.relative_url_root) + "/javascripts/workbox" %>",

Isso é exatamente o que queremos neste caso:

irb(main):001:0> puts "a#{nil}bc"
abc

Ah, eu quis dizer Discourse.base_path.

Acabei de fazer o commit de uma correção, por favor, dê uma olhada.

3 curtidas

Parece bom para o nosso caso… obrigado!

Mas… só para ter certeza… não tenho certeza de como vocês estão lidando com assets em uma CDN em combinação com uma subpasta, mas se você usa Discourse.asset_host, você ainda prefixa todos os caminhos no host de assets com o caminho da subpasta também? Porque é isso que o código está fazendo agora.
Se for isso que vocês estão fazendo, então podem ignorar completamente este parágrafo :slight_smile:

3 curtidas

Todo esse negócio de subpasta + CDN realmente causou muitos problemas. @featheredtoast fez um trabalho para simplificar isso e passamos bastante tempo garantindo que todo o nosso código de entrega de recursos funcione corretamente nessas combinações estranhas de buckets, subpastas, etc.

Acho que estamos seguros :smile:, mas, se necessário, podemos reabrir isso.

Obrigado pelo relatório do bug!

8 curtidas

Este tópico foi automaticamente fechado após 7 dias. Novas respostas não são mais permitidas.