Skip to content

Commit

Permalink
Fix compilation with DDC (#516)
Browse files Browse the repository at this point in the history
b272632 exposed client certificate through 
`X509Certificate? get clientCertificate;` getter on `ServiceCall` class. This broke 
compilation of `grpc_web` code using DDC, but not dart2js. Turns out that dart2js is 
happy to compile any code using `dart:io` (though the result will not run if you try
to use any of those APIs), but DDC rejects such code eagerly. `package:test` runs 
tests through `dart2js` so DDC breakage was not really caught by CI. 

Unfortunately this discrepancy between DDC and dart2js puts us in some really weird 
spot: most of our tests are platform independent, but most of those tests also 
pull in `dart:io` through transitive dependencies.

This commit is the most minimal change we could make to allow the code compile both 
on the Web and natively. 

A proper fix should be to go through tests one-by-one and make sure that those that
need to run on the Web don't import `dart:io`, but we don't have time to do that 
right now. 

This commit also adds a smoke test to the CI to verify that `grpc_web` example 
builds with DDC.
  • Loading branch information
mraleph authored Aug 16, 2021
1 parent c2fb47c commit 7cced92
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 11 deletions.
17 changes: 14 additions & 3 deletions .github/workflows/dart.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ jobs:
sdk: [dev, 2.12.0]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v1.0
- uses: dart-lang/setup-dart@v1.2
with:
sdk: ${{ matrix.sdk }}
- name: Report version
run: dart --version
- name: Install dependencies
run: dart pub get
- name: Check formatting
- name: Check formatting (using dev dartfmt release)
if: ${{ matrix.sdk == 'dev' }}
run: dart format --output=none --set-exit-if-changed .
- name: Analyze code (introp and examples)
run: |
Expand All @@ -39,6 +40,16 @@ jobs:
shell: bash
- name: Analyze code
run: dart analyze --fatal-infos .
- name: Check that grpc-web sample builds with DDC
if: ${{ matrix.sdk == 'dev' }}
# webdev build --no-release to force compilation with DDC.
run: |
dart pub global activate webdev
pushd example/grpc-web
rm -rf build
dart pub global run webdev build --no-release
test -f ./build/main.dart.js
popd
# Run tests on a matrix consisting of three dimensions:
# 1. OS: mac, windows, linux
Expand All @@ -61,7 +72,7 @@ jobs:
platform: chrome
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v1.0
- uses: dart-lang/setup-dart@v1.2
with:
sdk: ${{ matrix.sdk }}
- name: Report version
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 3.0.2

* Fix compilation on the Web with DDC.

## 3.0.1

* Require `package:googleapis_auth` `^1.1.0`
Expand Down
2 changes: 1 addition & 1 deletion lib/src/server/call.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:io';
import '../shared/io_bits/io_bits.dart' show X509Certificate;

/// Server-side context for a gRPC call.
///
Expand Down
2 changes: 1 addition & 1 deletion lib/src/server/handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@

import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:http2/transport.dart';

import '../shared/codec.dart';
import '../shared/codec_registry.dart';
import '../shared/io_bits/io_bits.dart' show X509Certificate;
import '../shared/message.dart';
import '../shared/status.dart';
import '../shared/streams.dart';
Expand Down
16 changes: 12 additions & 4 deletions lib/src/server/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import 'package:http2/transport.dart';
import 'package:meta/meta.dart';

import '../shared/codec_registry.dart';
import '../shared/io_bits/io_bits.dart' as io_bits;
import '../shared/security.dart';
import 'handler.dart';
import 'interceptor.dart';
Expand Down Expand Up @@ -129,8 +130,10 @@ class ConnectionServer {
ServerHandler_ serveStream_(ServerTransportStream stream,
[X509Certificate? clientCertificate]) {
return ServerHandler_(
lookupService, stream, _interceptors, _codecRegistry, clientCertificate)
..handle();
lookupService, stream, _interceptors, _codecRegistry,
// ignore: unnecessary_cast
clientCertificate as io_bits.X509Certificate?,
)..handle();
}
}

Expand Down Expand Up @@ -223,8 +226,13 @@ class Server extends ConnectionServer {
ServerHandler_ serveStream_(ServerTransportStream stream,
[X509Certificate? clientCertificate]) {
return ServerHandler_(
lookupService, stream, _interceptors, _codecRegistry, clientCertificate)
..handle();
lookupService,
stream,
_interceptors,
_codecRegistry,
// ignore: unnecessary_cast
clientCertificate as io_bits.X509Certificate?,
)..handle();
}

@Deprecated(
Expand Down
16 changes: 16 additions & 0 deletions lib/src/shared/io_bits/io_bits.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2021, the gRPC project authors. Please see the AUTHORS file
// for details. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

export 'io_bits_io.dart' if (dart.library.html) 'io_bits_web.dart';
16 changes: 16 additions & 0 deletions lib/src/shared/io_bits/io_bits_io.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2021, the gRPC project authors. Please see the AUTHORS file
// for details. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

export 'dart:io' show HttpStatus, X509Certificate;
20 changes: 20 additions & 0 deletions lib/src/shared/io_bits/io_bits_web.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) 2021, the gRPC project authors. Please see the AUTHORS file
// for details. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

export 'dart:html' show HttpStatus;

/// Should not be used on the Web, but is pulled through [ServiceCall] class
/// which is used in the protoc generated code.
class X509Certificate {}
2 changes: 1 addition & 1 deletion lib/src/shared/status.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// limitations under the License.

import 'dart:convert';
import 'dart:io' show HttpStatus;

import 'package:meta/meta.dart';
import 'package:protobuf/protobuf.dart';
Expand All @@ -23,6 +22,7 @@ import '../generated/google/protobuf/any.pb.dart';
import '../generated/google/rpc/code.pbenum.dart';
import '../generated/google/rpc/error_details.pb.dart';
import '../generated/google/rpc/status.pb.dart';
import 'io_bits/io_bits.dart' show HttpStatus;

class StatusCode {
/// The operation completed successfully.
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: grpc
description: Dart implementation of gRPC, a high performance, open-source universal RPC framework.

version: 3.0.1
version: 3.0.2

repository: https://github.com/grpc/grpc-dart

Expand Down

0 comments on commit 7cced92

Please sign in to comment.